diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -959,6 +959,8 @@ const LocationDescription &Loc, InsertPointTy AllocaIP, ArrayRef SectionCBs, PrivatizeCallbackTy PrivCB, FinalizeCallbackTy FiniCB, bool IsCancellable, bool IsNowait) { + assert(!isConflictIP(AllocaIP, Loc.IP) && "Dedicated IP allocas required"); + if (!updateToLocation(Loc)) return Loc.IP; diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp --- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp +++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp @@ -3548,7 +3548,12 @@ OMPBuilder.initialize(); F->setName("func"); IRBuilder<> Builder(BB); + + BasicBlock *EnterBB = BasicBlock::Create(Ctx, "sections.enter", F); + Builder.CreateBr(EnterBB); + Builder.SetInsertPoint(EnterBB); OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL}); + llvm::SmallVector SectionCBVector; llvm::SmallVector CaseBBs; @@ -3703,7 +3708,11 @@ F->setName("func"); IRBuilder<> Builder(BB); + BasicBlock *EnterBB = BasicBlock::Create(Ctx, "sections.enter", F); + Builder.CreateBr(EnterBB); + Builder.SetInsertPoint(EnterBB); OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL}); + IRBuilder<>::InsertPoint AllocaIP(&F->getEntryBlock(), F->getEntryBlock().getFirstInsertionPt()); llvm::SmallVector SectionCBVector; diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -66,7 +66,24 @@ if (walkResult.wasInterrupted()) return allocaInsertPoint; - // Otherwise, insert to the entry block of the surrounding function. + // Otherwise, insert to the entry block of the surrounding function.s + // If the current IRBuilder InsertPoint is the function's entry, it cannot + // also be used for alloca insertion which would result in insertion order + // confusion. Create a new BasicBlock for the Builder and use the entry block + // 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() == + &builder.GetInsertBlock()->getParent()->getEntryBlock()) { + assert(builder.GetInsertPoint() == builder.GetInsertBlock()->end() && + "Assuming end of basic block"); + llvm::BasicBlock *entryBB = llvm::BasicBlock::Create( + builder.getContext(), "entry", builder.GetInsertBlock()->getParent(), + builder.GetInsertBlock()->getNextNode()); + builder.CreateBr(entryBB); + builder.SetInsertPoint(entryBB); + } + llvm::BasicBlock &funcEntryBlock = builder.GetInsertBlock()->getParent()->getEntryBlock(); return llvm::OpenMPIRBuilder::InsertPointTy( @@ -249,24 +266,13 @@ // TODO: Is the Parallel construct cancellable? bool isCancellable = false; - // Ensure that the BasicBlock for the the parallel region is sparate from the - // function entry which we may need to insert allocas. - if (builder.GetInsertBlock() == - &builder.GetInsertBlock()->getParent()->getEntryBlock()) { - assert(builder.GetInsertPoint() == builder.GetInsertBlock()->end() && - "Assuming end of basic block"); - llvm::BasicBlock *entryBB = - llvm::BasicBlock::Create(builder.getContext(), "parallel.entry", - builder.GetInsertBlock()->getParent(), - builder.GetInsertBlock()->getNextNode()); - builder.CreateBr(entryBB); - builder.SetInsertPoint(entryBB); - } + llvm::OpenMPIRBuilder::InsertPointTy allocaIP = + findAllocaInsertPoint(builder, moduleTranslation); llvm::OpenMPIRBuilder::LocationDescription ompLoc( builder.saveIP(), builder.getCurrentDebugLocation()); builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createParallel( - ompLoc, findAllocaInsertPoint(builder, moduleTranslation), bodyGenCB, - privCB, finiCB, ifCond, numThreads, pbKind, isCancellable)); + ompLoc, allocaIP, bodyGenCB, privCB, finiCB, ifCond, numThreads, pbKind, + isCancellable)); return bodyGenStatus; } @@ -528,9 +534,10 @@ storeValues.push_back(vecValues[indexVecValues]); indexVecValues++; } + llvm::OpenMPIRBuilder::InsertPointTy allocaIP = + findAllocaInsertPoint(builder, moduleTranslation); builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createOrderedDepend( - ompLoc, findAllocaInsertPoint(builder, moduleTranslation), numLoops, - storeValues, ".cnt.addr", isDependSource)); + ompLoc, allocaIP, numLoops, storeValues, ".cnt.addr", isDependSource)); } return success(); } @@ -635,11 +642,13 @@ // called for variables which have destructors/finalizers. auto finiCB = [&](InsertPointTy codeGenIP) {}; + llvm::OpenMPIRBuilder::InsertPointTy allocaIP = + findAllocaInsertPoint(builder, moduleTranslation); llvm::OpenMPIRBuilder::LocationDescription ompLoc( builder.saveIP(), builder.getCurrentDebugLocation()); builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createSections( - ompLoc, findAllocaInsertPoint(builder, moduleTranslation), sectionCBs, - privCB, finiCB, false, sectionsOp.nowait())); + ompLoc, allocaIP, sectionCBs, privCB, finiCB, false, + sectionsOp.nowait())); return bodyGenStatus; } diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir --- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir +++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir @@ -842,6 +842,9 @@ // CHECK-LABEL: @omp_sections_trivial llvm.func @omp_sections_trivial() -> () { + // CHECK: br label %[[ENTRY:[a-zA-Z_.]+]] + + // CHECK: [[ENTRY]]: // CHECK: br label %[[PREHEADER:.*]] // CHECK: [[PREHEADER]]: