[OpenMP][Offloading] Fix test case issues in bug49334.cpp

[tianshilei1992]

Summary

bug49334.cpp has one issue that causes flaky result reported in #53730.
The root cause is BlockedC is never initialized but in BlockMatMul_TargetNowait
it is directly read and written (via +=). Fixes #53730.

[jhuber6]

LG

[hyviquel]

openmp/libomptarget/test/offloading/bug49334.cpp

76

@tianshilei1992 By changing N to 16, it uses only a single block thus becomes a simple matrix multiplication and the test lost the interest of having multiple concurent target tasks...

[tianshilei1992]

openmp/libomptarget/test/offloading/bug49334.cpp

76

Yes, but it can also make problems easier to be reproduced, like the one hidden in the test case itself. In addition, since the target region has nowait, one task or multiple tasks make no difference for the taskwait, which can meet our expectation for the test.

[tianshilei1992]

openmp/libomptarget/test/offloading/bug49334.cpp

76

I mean, the implicit task wait at the end of the parallel region.

[jdoerfert]

Arguably, we can have both, can't we?

[hyviquel]

openmp/libomptarget/test/offloading/bug49334.cpp

76

I think it is important to enable concurrent access to the same entry of the HostDataToTargetMap which was the initial purpose of the test.

[hyviquel]

In D119988#3341185, @jdoerfert wrote:

Arguably, we can have both, can't we?

I think it would be better.

[tianshilei1992]

I change it back to 256.

[protze.joachim]

In D119988#3341375, @tianshilei1992 wrote:

I change it back to 256.

I think what @jdoerfert and @hyviquel suggested is to make N a compile-time parameter and have two RUN lines, one setting -DN=16, one setting -DN=256.

openmp/libomptarget/test/offloading/bug49334.cpp

for (int i = 0; i < nBlocksPerCol; i++)
           for (int jj = 0; jj < rowsPerBlock; ++jj) {
             int curri = i * colsPerBlock + ii;
             int currj = j * rowsPerBlock + jj;
             CurrBlock[ii + jj * colsPerBlock] = matrix[curri + currj * nCols];
           }
       }
   }
 
-  long Compare(const std::vector<float> &matrix) const {
-    long fail = 0;
+  void Compare(const std::vector<float> &matrix) const {
     for (int i = 0; i < nBlocksPerCol; i++)
       for (int j = 0; j < nBlocksPerRow; j++) {
         float *CurrBlock = GetBlock(i, j);
         for (int ii = 0; ii < colsPerBlock; ++ii)
           for (int jj = 0; jj < rowsPerBlock; ++jj) {
             int curri = i * colsPerBlock + ii;
             int currj = j * rowsPerBlock + jj;
             float m_value = matrix[curri + currj * nCols];
             float bm_value = CurrBlock[ii + jj * colsPerBlock];
-            if (std::fabs(bm_value - m_value) >
-                std::numeric_limits<float>::epsilon()) {
-              fail++;
-            }
-          }
-      }
-    return fail;
-  }
+            assert(std::fabs(bm_value - m_value) <
+                   std::numeric_limits<float>::epsilon());
 
   float *GetBlock(int i, int j) const {
     assert(i < nBlocksPerCol && j < nBlocksPerRow && "Accessing outside block");
     return Blocks[i][j].get();
   }
 };
 
 constexpr const int BS = 16;
-constexpr const int N = 256;
+constexpr const int N = 16;
 
 int BlockMatMul_TargetNowait(BlockMatrix &A, BlockMatrix &B, BlockMatrix &C) {
 #pragma omp parallel
 #pragma omp master
   for (int i = 0; i < N / BS; ++i)
     for (int j = 0; j < N / BS; ++j) {
       float *BlockC = C.GetBlock(i, j);
       for (int k = 0; k < N / BS; ++k) {
int main(int argc, char *argv[]) {
 
   for (int i = 0; i < N; ++i) {
     for (int j = 0; j < N; ++j) {
       a[i * N + j] = b[i * N + j] = i + j % 100;
     }
   }
 
   auto BlockedA = BlockMatrix(BS, BS, N, N);
-  BlockedA.Initialize(a);
-  BlockedA.Compare(a);
   auto BlockedB = BlockMatrix(BS, BS, N, N);
+  auto BlockedC = BlockMatrix(BS, BS, N, N);
+  BlockedA.Initialize(a);
   BlockedB.Initialize(b);
+  BlockedC.Initialize(c);
+  BlockedA.Compare(a);
   BlockedB.Compare(b);
+  BlockedC.Compare(c);
 
   Matmul(a, b, c);
-
-  auto BlockedC = BlockMatrix(BS, BS, N, N);
   BlockMatMul_TargetNowait(BlockedA, BlockedB, BlockedC);
 
-  if (BlockedC.Compare(c) > 0) {
-    return 1;
-  }
+  BlockedC.Compare(c);
 
   std::cout << "PASS\n";
 
   return 0;
 }
 
 // CHECK: PASS