[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project.
Do you have commit rights already? If not I can commit this for you in a few days.



================
Comment at: llvm/lib/Frontend/OpenMP/OMPConstants.cpp:45
 ///{
-
 #define OMP_TYPE(VarName, InitValue) Type *llvm::omp::types::VarName = nullptr;
----------------
fghanim wrote:
> jdoerfert wrote:
> > Leftover
> what is this referring to?
You deleted an empty new line ;)


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:749
+    BasicBlock *ExitPredBB = SplitPos->getParent();
+    auto InsertBB = merged ? ExitPredBB : ExitBB;
+    if (!isa_and_nonnull<BranchInst>(SplitPos))
----------------
fghanim wrote:
> jdoerfert wrote:
> > I have the feeling the merging makes it harder without providing a benefit but I'm OK with it for now
> more difficult in what way?
We need things like `auto InsertBB = merged ? ExitPredBB : ExitBB;` now for example.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:686
+        break;
+      else
+        MasterEndCI = nullptr;
----------------
Nit: The `else` is not needed.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:745
+        break;
+      else
+        CriticalEntryCI = nullptr;
----------------
Nit: The `else` is not needed.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:762
+        break;
+      else
+        CriticalEndCI = nullptr;
----------------
Nit: The `else` is not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72304