jdoerfert marked 3 inline comments as done. jdoerfert added a comment. In D69922#1742392 <https://reviews.llvm.org/D69922#1742392>, @ABataev wrote:
> In D69922#1741659 <https://reviews.llvm.org/D69922#1741659>, @jdoerfert wrote: > > > Use IRBuilder for cancel barriers > > > In general, it would better to implement cancellation barriers in the > separate patch. Same for OpenMPIrBuilder. Why would you think that splitting these closely related concepts would be beneficial? In D69922#1751827 <https://reviews.llvm.org/D69922#1751827>, @kiranchandramohan wrote: > Thanks @jdoerfert for working on this. > > Sorry for being late to the party. I am trying to implement one trivial > directive (Flush) and one slightly more involved (not decided). > > I applied D69853 <https://reviews.llvm.org/D69853>, D69785 > <https://reviews.llvm.org/D69785>, D69922 <https://reviews.llvm.org/D69922> > to my local build and found that D69922 <https://reviews.llvm.org/D69922> is > referring to OpenMPIRBuilder.h in llvm/Frontend whereas in D69785 > <https://reviews.llvm.org/D69785> it was introduced in > llvm/Transforms/Utils/OpenMPIRBuilder.h The conversation (on the llvm-dev list) seems to have settled on `{include/llvm,lib}/frontend/OpenMP/`. I will move the files around once we finally settle these reviews that haven't moved (more than 2 comments at a time). As you can see, I have a huge backlog here and that is really hard to modify. You have a problem now as you depend on these. @hfinkel mentioned it in his email to the llvm-dev (about review etiquette), we need to minimize iterations in reviews and actually review patches not only a few lines every time. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3492 + CGF.createBasicBlock(".cancel.exit", IP.getBlock()->getParent()); + OMPBuilder->setCancellationBlock(ExitBB); + CGF.Builder.SetInsertPoint(ExitBB); ---------------- ABataev wrote: > Maybe, instead of saving the state, pass the pointer to the cancel block as a > parameter to the `CreateBarrier` function? All of this goes away in D70258, as it should. The frontend when creating a barrier does not need to know if it is cancelable and if so, where the cancel block is. 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
