[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via llvm-commits]

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


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+                     !OMPRegionInfo->hasCancel())) {
+    OMPBuilder->CreateBarrier({CGF.Builder.saveIP()}, Kind, ForceSimpleCall,
+                              EmitChecks);
----------------
ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > `createBarrier`
> > I'd say we align ourselves with the IRBuilder (which has `CreateXXX` methods) or we are different enough to avoid confusion, e.g., `emitXXXX`. I don't care much which way but `createXXXX` for one builder and `CreateXXXX` for another will be confusing, don't you think?
> Shall follow the LLVM coding document and format the new function name in accordance with it?
Changing the IRBuilder is something no one dared to do for years. I'm actually in favor but I would like to get this in rather sooner than later.

Two options I think are good:
  - Use `CreateXXX` now and, if someone ever comes around to rename the Builder methods we rename them in all Builders.
  - Use `emitXXXX` now to avoid confusion.

When someone in a review before asked for create instead of emit the reason was to be aligned with the IRBuilder.  I think that is a good justification so I went with `CreateXXX` even if it is against the coding standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922