Is there any better solution to initialize C arrays (created on the heap) fast? Like we do with curly brackets
double** matrix_multiply(const double **l_matrix, const double **r_matrix);
foo() {
double DCT_matrix[8][8] = {
{ 0.3536, 0.3536, 0.3536, 0.3536, 0.3536, 0.3536, 0.3536, 0.3536 },
{ 0.4904, 0.4157, 0.2778, 0.0975, -0.0975, -0.2778, -0.4157, -0.4904 },
{ 0.4619, 0.1913, -0.1913, -0.4619, -0.4619, -0.1913, 0.1913, 0.4619 },
{ 0.4157, -0.0975, -0.4904, -0.2778, 0.2778, 0.4904, 0.0975, -0.4157 },
{ 0.3536, -0.3536, -0.3536, 0.3536, 0.3536, -0.3536, -0.3536, 0.3536 },
{ 0.2778, -0.4904, 0.0975, 0.4157, -0.4157, -0.0975, 0.4904, -0.2778 },
{ 0.1913, -0.4619, 0.4619, -0.1913, -0.1913, 0.4619, -0.4619, 0.1913 },
{ 0.0975, -0.2778, 0.4157, -0.4904, 0.4904, -0.4157, 0.2778, -0.0975 }
};
const double other_matrix[8][8] = {
{ 26, -5, -5, -5, -5, -5, -5, 8 },
{ 64, 52, 8, 26, 26, 26, 8, -18 },
{ 126, 70, 26, 26, 52, 26, -5, -5 },
{ 111, 52, 8, 52, 52, 38, -5, -5 },
{ 52, 26, 8, 39, 38, 21, 8, 8 },
{ 0, 8, -5, 8, 26, 52, 70, 26 },
{ -5, -23, -18, 21, 8, 8, 52, 38 },
{ -18, 8, -5, -5, -5, 8, 26, 8 }
};
matrix_multiply(DCT_matrix, other_matrix); // Segfault
}
CodePudding user response:
In the comment section, it has become clear that the main flaw is the design of matrix_multiply
. There's no reason to pass double**
to it.
Firstly, the layout of double**
indicates that the matrix is created like this:
double **mat = malloc(8 * sizeof *mat);
for(int i=0; i<8; i )
mat[i] = malloc(8 * sizeof *mat[0]);
This is how this is taught in introductory classes when learning how to deal with pointers. But this causes many unnecessary malloc calls, which also will make it clunky to free all the memory. On top of that, it is slow. Both while allocating/freeing and while working with the memory since the matrix will not be cache friendly. Here is an answer I wrote that is about the cache
I have a few others here
So, in your function, I assume you will have a lot of stuff like l_matrix[x][y]
. Replace this with l_matrix[x 8*y]
when going from double**
to double*
.
On top of that, since you have no output argument, I assume that you allocate the output inside the function. Don't do that. There are many situations when you want to reuse memory allocated for matrices. Instead, provide an output argument and allocate the memory outside the function. This gives us the signature:
void matrix_multiply(const double *A, const double *B, double *output)
Also, since in your case the matrix size is fixed, I would recommend renaming the function in some way that reflects that. Maybe DCT_matrix_multiply
but that's up to you.
If you want, you can change the return type to double*
and end the function with return output;
. That's not mandatory, but it gives the option of doing this:
matrix_multiply(matrix_multiply(A, B, C),
matrix_multiply(A, D, E),
F);
However, be VERY careful if you do that, because the evaluation order of arguments is not specified.
If you really want a function that does the allocation, then write a wrapper:
double *wrapper(const double *A, const double *B) {
double *output = malloc(8*8 * sizeof *output);
if(!output) return NULL;
return matrix_multiply(A, B, output);
}
If you want to work with matrices with non fixed size, it's a bit trickier, but in general, I would still recommend passing double*
with added information about sizes. Something like:
double *mul(const double *A, const double *B, double *C, size_t x, size_t y);
If you want to support multiplication with matrices and vectors, the signature will become even more complicated, and the complete solution for this is outside the scope of this answer. But I would write a separate function in such a case.
Another option you might want to investigate is having this signature.
typedef dm double[8][8]; // Short for dct matrix
dm *matrix_multiply(const dm *A, const *dm B, dm *output);
It makes some sense here since you're working with fixed sizes, but in general, this approach is not very common. The reason is simply that the benefits of it doesn't really outweigh the drawbacks of having to deal with pointers to 2d-arrays.
Workaround to your current code
If you want to postpone the rewriting but still be able to initialize the way you want, write a converter.
double **convert (double mat[8][8])
{
double **ret = malloc (8 * sizeof *ret);
if (!ret) return NULL;
for (int i = 0; i < 8; i )
ret[i] = &mat[i][0];
return ret;
}
Then you can do
double **A = convert(DCT_matrix);
double **B = convert(other_matrix);
// Use A and B
free(A);
free(B);