[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked an inline comment as done.
jdoerfert added a comment.

I'll address the nits. Unsure if that is all. I also don't get the one comment.



================
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:
> 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. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82470/new/

https://reviews.llvm.org/D82470