taco generates invalid "#pragma omp atomic" for Store ops

[Infinoid]

In the scheduling_eval.test_spmvCPU_temp test, Taco generates the following code:

    #pragma omp atomic
    y_vals[i] = tjy_val;

On Debian bullseye (testing), this fails to compile with the following error:

[ RUN      ] scheduling_eval.test_spmvCPU_temp
/tmp/taco_tmp_4Ids2M/1wry7ti8f7ci.c: In function ‘compute’:
/tmp/taco_tmp_4Ids2M/1wry7ti8f7ci.c:137:24: error: invalid form of ‘#pragma omp atomic’ before ‘;’ token
  137 |     y_vals[i] = tjy_val;
      |                        ^
Error at /home/infinoid/workspace/taco/taco/src/codegen/module.cpp:154 in compile:
 Compilation command failed:
gcc-10 -O3 -ffast-math -std=c99 -shared -fPIC -fopenmp /tmp/taco_tmp_4Ids2M/1wry7ti8f7ci.c  -o /tmp/taco_tmp_4Ids2M/1wry7ti8f7ci.so -lm
returned 256
[  FAILED  ] scheduling_eval.test_spmvCPU_temp (190 ms)

The test passes on ubuntu-20.04 and ubuntu-18.04. I think they added some new checks in the new compiler version. (gcc and libgomp1 version 10.2.0)

git bisect indicates that the test starts failing in e976ecb. Prior to that commit, the code generated was:

    #pragma omp atomic
    y_vals[i] = y_vals[i] + tj_val;

This compiles successfully; the new syntax does not.

This Store op doesn't appear to need atomics. Each thread has different values of i, and this is the only place y_vals[i] is accessed, so the accesses should be safe. But if it does need it, adding a "write" at the end (#pragma omp atomic write) is enough to make the compiler happy again.

For convenience, I posted generated source files for current and "last known working" versions at https://gist.github.com/Infinoid/1692a5963997fa5bccbaf42b0b29cc67 .

[Infinoid]

I also see this failure on ubuntu 16.04 with gcc/libgomp1 version 5.4.0.

[Infinoid]

adding a "write" at the end (#pragma omp atomic write) is enough to make the compiler happy again.

This is definitely not the right fix... it causes other tests to fail.

[stephenchouca]

This should be fixed in the latest commit on master. Let me know if there's some issue with the fix.

[Infinoid]

Yep, 0a48d17 fixed this. #402 adds a test case to ensure that atomics are retained in cases where they are still needed.