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