diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -1441,6 +1441,50 @@ return UDRMap.lookup(D); } +// Temporary RAII solution to perform a push/pop stack event on the OpenMP IR +// Builder if one is present. +struct PushAndPopStackRAII { + PushAndPopStackRAII(llvm::OpenMPIRBuilder *OMPBuilder, CodeGenFunction &CGF, + bool HasCancel) + : OMPBuilder(OMPBuilder) { + if (!OMPBuilder) + return; + + // The following callback is the crucial part of clangs cleanup process. + // + // NOTE: + // Once the OpenMPIRBuilder is used to create parallel regions (and + // similar), the cancellation destination (Dest below) is determined via + // IP. That means if we have variables to finalize we split the block at IP, + // use the new block (=BB) as destination to build a JumpDest (via + // getJumpDestInCurrentScope(BB)) which then is fed to + // EmitBranchThroughCleanup. Furthermore, there will not be the need + // to push & pop an FinalizationInfo object. + // The FiniCB will still be needed but at the point where the + // OpenMPIRBuilder is asked to construct a parallel (or similar) construct. + auto FiniCB = [&CGF](llvm::OpenMPIRBuilder::InsertPointTy IP) { + assert(IP.getBlock()->end() == IP.getPoint() && + "Clang CG should cause non-terminated block!"); + CGBuilderTy::InsertPointGuard IPG(CGF.Builder); + CGF.Builder.restoreIP(IP); + CodeGenFunction::JumpDest Dest = + CGF.getOMPCancelDestination(OMPD_parallel); + CGF.EmitBranchThroughCleanup(Dest); + }; + + // TODO: Remove this once we emit parallel regions through the + // OpenMPIRBuilder as it can do this setup internally. + llvm::OpenMPIRBuilder::FinalizationInfo FI( + {FiniCB, OMPD_parallel, HasCancel}); + OMPBuilder->pushFinalizationCB(std::move(FI)); + } + ~PushAndPopStackRAII() { + if (OMPBuilder) + OMPBuilder->popFinalizationCB(); + } + llvm::OpenMPIRBuilder *OMPBuilder; +}; + static llvm::Function *emitParallelOrTeamsOutlinedFunction( CodeGenModule &CGM, const OMPExecutableDirective &D, const CapturedStmt *CS, const VarDecl *ThreadIDVar, OpenMPDirectiveKind InnermostKind, @@ -1465,6 +1509,11 @@ else if (const auto *OPFD = dyn_cast(&D)) HasCancel = OPFD->hasCancel(); + + // TODO: Temporarily inform the OpenMPIRBuilder, if any, about the new + // parallel region to make cancellation barriers work properly. + llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder(); + PushAndPopStackRAII PSR(OMPBuilder, CGF, HasCancel); CGOpenMPOutlinedRegionInfo CGInfo(*CS, ThreadIDVar, CodeGen, InnermostKind, HasCancel, OutlinedHelperName); CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, &CGInfo); @@ -3488,21 +3537,8 @@ dyn_cast_or_null(CGF.CapturedStmtInfo); llvm::OpenMPIRBuilder *OMPBuilder = CGF.CGM.getOpenMPIRBuilder(); if (OMPBuilder) { - // TODO: Move cancelation point handling into the IRBuilder. - if (EmitChecks && !ForceSimpleCall && OMPRegionInfo && - OMPRegionInfo->hasCancel() && CGF.Builder.GetInsertBlock()) { - CGBuilderTy::InsertPointGuard IPG(CGF.Builder); - llvm::BasicBlock *ExitBB = CGF.createBasicBlock( - ".cancel.exit", CGF.Builder.GetInsertBlock()->getParent()); - OMPBuilder->setCancellationBlock(ExitBB); - CGF.Builder.SetInsertPoint(ExitBB); - CodeGenFunction::JumpDest CancelDestination = - CGF.getOMPCancelDestination(OMPRegionInfo->getDirectiveKind()); - CGF.EmitBranchThroughCleanup(CancelDestination); - } - auto IP = OMPBuilder->CreateBarrier(CGF.Builder, Kind, ForceSimpleCall, - EmitChecks); - CGF.Builder.restoreIP(IP); + CGF.Builder.restoreIP(OMPBuilder->CreateBarrier( + CGF.Builder, Kind, ForceSimpleCall, EmitChecks)); return; } diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h @@ -37,12 +37,44 @@ /// Add attributes known for \p FnID to \p Fn. void addAttributes(omp::RuntimeFunction FnID, Function &Fn); - /// Set the cancellation block to \p CBB. - void setCancellationBlock(BasicBlock *CBB) { CancellationBlock = CBB; } - /// Type used throughout for insertion points. using InsertPointTy = IRBuilder<>::InsertPoint; + /// Callback type for variable finalization (think destructors). + /// + /// \param CodeGenIP is the insertion point at which the finalization code + /// should be placed. + /// + /// A finalize callback knows about all objects that need finalization, e.g. + /// destruction, when the scope of the currently generated construct is left + /// at the time, and location, the callback is invoked. + using FinalizeCallbackTy = std::function; + + struct FinalizationInfo { + /// The finalization callback provided by the last in-flight invocation of + /// CreateXXXX for the directive of kind DK. + FinalizeCallbackTy FiniCB; + + /// The directive kind of the innermost directive that has an associated + /// region which might require finalization when it is left. + omp::Directive DK; + + /// Flag to indicate if the directive is cancellable. + bool IsCancellable; + }; + + /// Push a finalization callback on the finalization stack. + /// + /// NOTE: Temporary solution until Clang CG is gone. + void pushFinalizationCB(const FinalizationInfo &FI) { + FinalizationStack.push_back(FI); + } + + /// Pop the last finalization callback from the finalization stack. + /// + /// NOTE: Temporary solution until Clang CG is gone. + void popFinalizationCB() { FinalizationStack.pop_back(); } + /// Description of a LLVM-IR insertion point (IP) and a debug/source location /// (filename, line, column, ...). struct LocationDescription { @@ -112,6 +144,19 @@ omp::Directive DK, bool ForceSimpleCall, bool CheckCancelFlag); + /// The finalization stack made up of finalize callbacks currently in-flight, + /// wrapped into FinalizationInfo objects that reference also the finalization + /// target block and the kind of cancellable directive. + SmallVector FinalizationStack; + + /// Return true if the last entry in the finalization stack is of kind \p DK + /// and cancellable. + bool isLastFinalizationInfoCancellable(omp::Directive DK) { + return !FinalizationStack.empty() && + FinalizationStack.back().IsCancellable && + FinalizationStack.back().DK == DK; + } + /// Return the current thread ID. /// /// \param Ident The ident (ident_t*) describing the query origin. @@ -123,9 +168,6 @@ /// The LLVM-IR Builder used to create IR. IRBuilder<> Builder; - /// TODO: Stub for a cancellation block stack. - BasicBlock *CancellationBlock = nullptr; - /// Map to remember source location strings StringMap SrcLocStrMap; 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 @@ -205,7 +205,8 @@ // If we are in a cancellable parallel region, barriers are cancellation // points. // TODO: Check why we would force simple calls or to ignore the cancel flag. - bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock; + bool UseCancelBarrier = + !ForceSimpleCall && isLastFinalizationInfoCancellable(OMPD_parallel); Value *Result = Builder.CreateCall( getOrCreateRuntimeFunction(UseCancelBarrier ? OMPRTL___kmpc_cancel_barrier @@ -217,19 +218,22 @@ BasicBlock *BB = Builder.GetInsertBlock(); BasicBlock *NonCancellationBlock = BasicBlock::Create( BB->getContext(), BB->getName() + ".cont", BB->getParent()); + BasicBlock *CancellationBlock = BasicBlock::Create( + BB->getContext(), BB->getName() + ".cncl", BB->getParent()); // Jump to them based on the return value. Value *Cmp = Builder.CreateIsNull(Result); Builder.CreateCondBr(Cmp, NonCancellationBlock, CancellationBlock, /* TODO weight */ nullptr, nullptr); - Builder.SetInsertPoint(NonCancellationBlock); - assert(CancellationBlock->getParent() == BB->getParent() && - "Unexpected cancellation block parent!"); + // From the cancellation block we finalize all variables and go to the + // post finalization block that is known to the FiniCB callback. + Builder.SetInsertPoint(CancellationBlock); + auto &FI = FinalizationStack.back(); + FI.FiniCB(Builder.saveIP()); - // TODO: This is a workaround for now, we always reset the cancellation - // block until we manage it ourselves here. - CancellationBlock = nullptr; + // The continuation block is where code generation continues. + Builder.SetInsertPoint(NonCancellationBlock); } return Builder.saveIP(); 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 @@ -104,7 +104,15 @@ BasicBlock *CBB = BasicBlock::Create(Ctx, "", F); new UnreachableInst(Ctx, CBB); - OMPBuilder.setCancellationBlock(CBB); + auto FiniCB = [CBB](llvm::OpenMPIRBuilder::InsertPointTy IP) { + assert(IP.getBlock()->end() == IP.getPoint() && + "Clang CG should cause non-terminated block!"); + BranchInst::Create(CBB, IP.getBlock()); + }; + // Emulate an outer parallel. + llvm::OpenMPIRBuilder::FinalizationInfo FI( + {FiniCB, OMPD_parallel, /* HasCancel */ true}); + OMPBuilder.pushFinalizationCB(std::move(FI)); IRBuilder<> Builder(BB); @@ -113,7 +121,7 @@ Builder.restoreIP(NewIP); EXPECT_FALSE(M->global_empty()); EXPECT_EQ(M->size(), 3U); - EXPECT_EQ(F->size(), 3U); + EXPECT_EQ(F->size(), 4U); EXPECT_EQ(BB->size(), 4U); CallInst *GTID = dyn_cast(&BB->front()); @@ -133,10 +141,15 @@ Instruction *BarrierBBTI = Barrier->getParent()->getTerminator(); EXPECT_EQ(BarrierBBTI->getNumSuccessors(), 2U); EXPECT_EQ(BarrierBBTI->getSuccessor(0), NewIP.getBlock()); - EXPECT_EQ(BarrierBBTI->getSuccessor(1), CBB); + EXPECT_EQ(BarrierBBTI->getSuccessor(1)->getTerminator()->getNumSuccessors(), + 1U); + EXPECT_EQ(BarrierBBTI->getSuccessor(1)->getTerminator()->getSuccessor(0), + CBB); EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID); + OMPBuilder.popFinalizationCB(); + Builder.CreateUnreachable(); EXPECT_FALSE(verifyModule(*M)); }