1

I'm writing code for matrix multiplication of 2 matrices, with sizes n1,n2 and n2,n3.

This is what I tried:

#include <climits>
#include <iostream>
using namespace std;

int main() {
    int n1 = 3;
    int n2 = 3;
    int n3 = 3;
    
    int arr1[n1][n2] = {
        {1, 2, 3},
        {5, 6, 7},
        {9, 10, 11}
    };
    int arr2[n2][n3] = {
        {2, 0, 0},
        {0, 2, 0},
        {0, 0, 2}
    };
    int res[n1][n3];

    for (int c = 0; c < n3; c++) {
        for (int r = 0; r < n1; r++) {
            for (int i = 0; i < n2; i++) {
                res[r][c] += arr1[r][i] * arr2[i][c];  // This is the main part
            }
        }
    }

    for (int i = 0; i < n1; i++) {
        for (int j = 0; j < n3; j++) {
            cout << res[i][j] << ' ';
        }
        cout << endl;
    }

    return 0;
}

The output was:

6422058 1877910233 1878006934 
1878000406 20 7022734 
7022746 68 22 

Then I tried to change the line which had +=:

int res[n1][n3];

for(int c = 0; c < n3; c++){
    for(int r = 0; r < n1; r++){
        int val = 0;
        for(int i = 0; i < n2; i++){
            val += arr1[r][i] * arr2[i][c];    // Here I used new variable val
        }
        res[r][c] = val;
    }
}

The problem disappeared. Output:

2 4 6 
10 12 14 
18 20 22 

Why did this fix the problem?

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
  • 3
    Your code uses *variable-length arrays*, and such arrays are not allowed in C++. Use `std::vector` if the sizes are not known at compile-time (or needs to be resized at runtime). Or use `const` or `constexpr` values for the array sizes. – Some programmer dude Jun 30 '23 at 11:03
  • 5
    `int res[n1][n3]` is not even a valid C++; Even if `n1` and `n3` are constants, the array remains uninitialized, so your program output is undefined. – pptaszni Jun 30 '23 at 11:04
  • 4
    Also the arrays `res` are not initialized. Each element will have *indeterminate* values, and using them in any way (like you're doing with `+=`) will lead to *undefined behavior*. – Some programmer dude Jun 30 '23 at 11:05
  • 1
    Side note: About [`using namespace std`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – Aconcagua Jun 30 '23 at 11:06
  • 1
    About VLA: Not legal in C++, but why do they still compile??? They are legal in C and many C++ compilers are able to compile C as well; they then import the VLA feature from C to C++ as an *extension*. Using VLA, though, you rely on such compilers, your code is not fully portable. If you happen to compile with GCC or clang I'd recommend adding `-Werror=vla` to the compiler flags to avoid accidental use of VLA. – Aconcagua Jun 30 '23 at 11:15

1 Answers1

3

The problem is that on the line:

int res[n1][n3];

all of the memory inside of res is going to be indeterminate, not zero. That's why such nonsensical and large values are being printed out. It's a symptom of undefined behavior in your code.

int val = 0;
for(int i=0; i < n2; i++){
    val += arr1[r][i] * arr2[i][c];    // Here I used new variable val
}
res[r][c] = val;

This fixes it, because you start with val = 0, then add onto it, and write the result to res[r][c], so there is no undefined behavior here. However, you could have also just written:

int res[n1][n3] {};
// or
int res[n1][n3] = {};
// or
int res[n1][n3] = {0}; // C-compatible

... at the top, and this would have initialized all of the elements in res to zero. All of these do the same thing.

Note on VLAs

All of your arrays are Variable Length Arrays (VLAs), which is not allowed in C++. You would have to make their sizes constexpr, or at least const to fix this:

constexpr int n1 = 3;
constexpr int n2 = 3;
constexpr int n3 = 3;

Your code only compiles because the compiler supports VLAs as a non-standard extension.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
  • 1
    @Akshat Just for the case you ever see (or even prefer yourself…): `int res[n1][n3] = { }` is just as fine, doing exactly the same; it's older syntax from pre-C++11 times. – Aconcagua Jun 30 '23 at 11:10
  • Thank you so much. I didn't know about this VLA thing, which is also why I was confused that even though I am not using heap memory my arrays have variable size. – Akshat Shahjade Jun 30 '23 at 13:21