[Openmp-commits] [PATCH] D64218: [OpenMP][NFCI] Cleanup the target synchronization implementation

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:48
+///        need to implement it differently
+template <bool IsCancellable, bool IsSimple, bool IsSPMD>
+void __kmpc_impl_barrier(kmp_Ident *Loc, int32_t TID) {
----------------
ABataev wrote:
> I don't think this is correct. `IsSPMD` flag should be passed as a function parameter. Sometimes, we cannot define the execution mode at the compile time and we could define it only at the execution time (foe example, if the parallel region is called in the orphaned function, marked as noinline or compiled without optimizations, etc.)
It is "correct" and it "works" with the rest of the code base but we can change it regardless:

It works this way because we have explicit `__kmpc_barrier_XXXXX` functions for the SPMD and non-SPMD case. Through that level of abstraction we know the required barrier implemenetation at compile time. If we want to move avay from the different barrier types that have the mode baked into their name, we would need to make the template parameters arguments for sure.

Long story short, I do not have strong feelings about this and it should not matter after inlining and constant propagation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64218/new/

https://reviews.llvm.org/D64218