[flang][openmp] crash in complex atomic

[jeanPerier]

  complex c
  integer :: i
  c=(1.0,1.0)
  call omp_set_num_threads(4)
  !$omp parallel do
     do i = 1, 100000000
     !$omp atomic
     c = c + (1.0,1.0)
  end do
  print *, c
end

Segfaults in __atomic_compare_exchange

#0  0x00007ffff7fa46c7 in __atomic_compare_exchange () from /lib/x86_64-linux-gnu/libatomic.so.1
#1  0x0000555555555418 in _QQmain..omp_par () at repro.f90:7
#2  0x00007ffff62c3239 in __kmp_invoke_microtask () from lib/libomp.so
#3  0x00007ffff624041f in __kmp_invoke_task_func () from lib/libomp.so
#4  0x00007ffff623f07e in __kmp_launch_thread () from lib/libomp.so
#5  0x00007ffff62a2f18 in __kmp_launch_worker(void*) () from lib/libomp.so
#6  0x00007ffff5e94ac3 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#7  0x00007ffff5f26660 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

If I lower the loop iteration from 100000000 to 1000000, I do not see such crash and the expected number is printed.

[jeanPerier]

FYI @NimishMishra

[jeanPerier]

FYI, I think this is because of stack overflow because there is an alloca generated for the atomic load inside the loop.

From the -fc1 -emit-llvm -fopenmp output of the repro above, I see:

omp.loop_nest.region:                             ; preds = %omp_loop.body
  store i32 %8, ptr %omp.private.alloc, align 4
  %atomic.temp.load = alloca { float, float }, align 8
  call void @__atomic_load(i64 8, ptr %loadgep_, ptr %atomic.temp.load, i32 0)
  %9 = load { float, float }, ptr %atomic.temp.load, align 8
  br label %.atomic.cont

I think the alloca should be outside of the loop or there should be a stack save/restore generated.
This seems to be generated late (in the OpenMP IR builder?) because I do not see this alloca in the -emit-fir output.

[kiranchandramohan]

It is probably being added in

llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Line 8810 in b7eb01b

AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);

llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Line 8859 in b7eb01b

AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);

There are some changes in related areas in #131603. May be the same comments of adding stack save and restore are applicable there as well.

[NimishMishra]

I think the alloca should be outside of the loop or there should be a stack save/restore generated. This seems to be generated late (in the OpenMP IR builder?) because I do not see this alloca in the -emit-fir output.

Thanks @jeanPerier. Yes this is indeed the problem. We have a potential solution here: #101966, which should simply how we emit the libcalls.

[NimishMishra]

AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);
There are some changes in related areas in #131603. May be the same comments of adding stack save and restore are applicable there as well.

Yes right. I did not consider this; we are generating allocas here too.

[NimishMishra]

I have linked #132888 as a fix to this issue, since the other PR might take a while to land (it has dependency on Clang).

With the fix, flang outputs (1.6777216E+07, 1.6777216E+07), which is the expected output.