[OpenMP][DeviceRTL] Fixed an issue that causes hang in SU3

[tianshilei1992]

Summary

The synchronization at the end of parallel region cannot make sure all threads
exit the scope. As a result, the assertions right after it might be hit, and
further the state::assumeInitialState(IsSPMD) in __kmpc_target_deinit may
not hold as well. We either add a synchronization right after the parallel region,
or remove the assertions and assuptions. Here we choose the first one as those
assertions and assumptions can help optimizations.

[jdoerfert]

LG

[ye-luo]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

This explanation seems plausible. Are the synchronization requirement of ASSERT and why being documented somewhere?

[tianshilei1992]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

Have no idea.

[ye-luo]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

synchronize::threadsAligned() has been called at the end of above scope.
What caused the out of sync and needs this additional call?
Why ASSERT requires synchronization.
All needs explanation or documentation.
"Synchronize all threads to make sure every thread exits the scope above;"
Just sounds plausible

[ye-luo]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

Making code slower by the addtional synchronization can also potentially skip triggering the hang. So it needs to be well understood and explained.

[tianshilei1992]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

The key point here is the scope because state configuration is via RAII. The synchronization at the end of the scope can only make sure threads are aligned *before* exiting the scope, but since then, it is undetermined which threads exits first.

[ye-luo]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

If the destructor sets a state which is supposed to be shared by all the threads, can synchronize::threadsAligned() be called inside the destructor?

[ye-luo]

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

129

OK. The destructor is only for a single value and having one alignment for multiple destructors is beneficial.

openmp/libomptarget/DeviceRTL/src/Parallelism.cpp

synchronize::threadsAligned();
 
       if (TId < NumThreads)
         invokeMicrotask(TId, 0, fn, args, nargs);
 
       // Synchronize all threads at the end of a parallel region.
       synchronize::threadsAligned();
     }
 
+    // Synchronize all threads to make sure every thread exits the scope above;
+    // otherwise the following assertions and the assumption in
+    // __kmpc_target_deinit may not hold.
+    synchronize::threadsAligned();
+
     ASSERT(state::ParallelTeamSize == 1u);
     ASSERT(icv::ActiveLevel == 0u);
     ASSERT(icv::Level == 0u);
     return;
   }
 
   // We do *not* create a new data environment because all threads in the team
   // that are active are now running this parallel region. They share the