[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

Fri Aug 13 12:06:47 PDT 2021

Meinersbur added a comment.

While manually editing `ordered_codegen.cpp` gives nicer results, re-running `update_cc_test_checks.py` would be less work. Your manual changes would be also be dropped the next time somebody runs update_cc_test_checks.py and commits.

The code seems derived from @fghanim single/master/etc implementation, would be nice if they could have a look.

The non-OMPBuilder code emits an outlined `__captured_stmt`, but not with OMPBuilder enabled. I assume this is due to the missing `finatlize` call.

================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5335
+                              llvm::BasicBlock &FiniBB) {
+        // TODO: The ordered directive with simd clause.
+
----------------
Also, out and `assert`/`llvm_unreachable` here?

================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5323-5325
+        Range = AssertSuccess(Actions.BuildBinOp(
+            nullptr, {}, BO_Add, Range,
+            Actions.ActOnIntegerConstant(SourceLocation(), 1).get()));
----------------
Haven't really understood why this is necessary, but this new version LGTM.

================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2154
+  Builder.restoreIP(
+      OMPBuilder.createOrdered(Builder, BodyGenCB, FiniCB, false));
+
----------------
Did you intend to pass IsThreads=true. Currently it is failing because no `__kmpc_ordered` is emitted.

================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2156-2157
+
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_EQ(EntryBBTI, nullptr);
+
----------------
Consider emitting a terminator, call `finalize()` and `verifyModule`.

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

https://reviews.llvm.org/D107430