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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to