[PATCH] D117835: [OpenMPIRBuilder] Detect and fix ambiguous InsertPoints for createSections.

Mon Apr 4 20:18:44 PDT 2022

Meinersbur marked an inline comment as done.
Meinersbur added inline comments.

================
Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:74-75
+  // for the allocs.
+  // TODO: Create a dedicated alloca BasicBlock at function creation such that
+  // we do not need to move the current InertPoint here.
+  if (builder.GetInsertBlock() ==
----------------
shraiysh wrote:
> [Suggestion] We probably don't need to do this, because if a conversion does not require an alloca (for example, barrier), we will be creating a basicblock unnecessarily. So, creating this on-demand seems okay to me.
Making decision based on the current position of an IRBuilder creates heaps of problems and unpredictability. Special cases like this one here need to be tested and maintained, and still easily introduce bugs. An unconditional dedicated alloca block avoids all these problems.

An extra basic block on the other side is insignificant. It will be gone as soon as simplify-cfg runs.

However, this seems to be controversial, so I removed the TODO for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117835