jdoerfert marked an inline comment as done. jdoerfert added inline comments.
================ Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:433 - Builder.SetInsertPoint(OuterFn->getEntryBlock().getFirstNonPHI()); - AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr"); - AllocaInst *ZeroAddr = Builder.CreateAlloca(Int32, nullptr, "zero.addr"); + IRBuilder<> AllocaBuilder(AllocaIP.getBlock(), AllocaIP.getPoint()); + ---------------- fghanim wrote: > jdoerfert wrote: > > fghanim wrote: > > > Here and elsewhere: You seem to have forgotten to undo the changes > > > introduced by using a separate `AllocaBuilder`? > > > > > > Also, Nit: before using `AllocaIP`, either add an assert that it is not > > > empty, or alternatively, add logic to just set it to entry block of > > > `outerfn` if it is > > > Here and elsewhere: You seem to have forgotten to undo the changes > > > introduced by using a separate AllocaBuilder? > > > > I don't get this. These changes are on purpose. > Oh, my bad. I assumed that since we now pass `allocaip` to communicate where > is the insertion point, using `builder` the way we used to is sufficient, and > this was leftover code. So now what purpose does `AllocaBuilder` serve? 1) No switching the IP back and forth in one builder all the time. 2) Showing by the name of the builder directly where the instructions will be created. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82470/new/ https://reviews.llvm.org/D82470 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
