[PATCH] D104928: [OMPIRBuilder] add minimalist reduction support

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Some cases that don't match LLVM style (`method`, `function`, `guard`). However, I think loop iterator cases (`e`, `i`, ...) are rather a problem with clang-tidy.

`createReductions` takes many arrays. How feasible is it to make it emit just one reduction per call, so front-ends would call it once per reduction? In the current form, would it be even legal to it multiple times with just one variable to reduce?



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:514
+  ///   type var_2;
+  ///   #pragma omp reduction(reduction-op:var_1,var_2)
+  ///   /* body */;
----------------
The reduction clause would need to be attached to a directive


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1104-1106
+  // BasicBlock *ContinuationBlock =
+  //     BasicBlock::Create(module->getContext(), "reduce.switch.cont",
+  //     function);
----------------
Remove commented-out code entirely?


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2522-2523
 
+// Returns the single instruction of InstTy type in BB that uses the value V. If
+// there is more than one such instruction, returns null.
+template <typename InstTy>
----------------
Consider using doxygen comments.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2749
+  GlobalVariable *LockVar =
+      M->getGlobalVariable(".gomp_critical_user_.reduction.var");
+  EXPECT_NE(LockVar, nullptr);
----------------
Why `gomp` ("GNU OpenMP") in the name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104928