fghanim added a comment. > I'll address the nits.
Thanks :) > Unsure if that is all. depends on the comment about `AllocaBuilder` ================ 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()); + ---------------- 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? 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
