[PATCH] D128203: [OpenMP][IRBuilder] Add support for taskgroup

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

Using it in Clang as well would certainly be nice in agreement with @peixin, I don't think required to get this patch in, or for the Flang implementation to wait until there is a use in Clang.

So LGTM. You could wait with committing until you have the Clang patch that verifies that it actually works end-to-end, but it is up to you.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:633
 
+  /// Generator for `#omp taskgroup`
+  ///
----------------
[nit] Or using Fortran syntax. Or more generally "taskgroup construct"


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:4968
+  // Checking the general structure of the IR generated is same as expected.
+  Instruction *GeneratedStoreInst = TaskgroupCall->getNextNonDebugInstruction();
+  EXPECT_EQ(GeneratedStoreInst, InternalStoreInst);
----------------
While I dislike making the tests fragile to depend on the instruction order in the BB, I think this limited use is still acceptable.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:4982
+                            ElseTerm->getParent()};
+  verifyDFSOrder(F, RefOrder);
+}
----------------
So glad you are using this function instead of walking the branch instructions! 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128203