From 957d856e7e02de9b172c943c76b6d5caeef31d9b Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Thu, 17 Nov 2016 15:12:05 +0000 Subject: [PATCH] [OPENMP] Fixed codegen for 'omp cancel' construct. If 'omp cancel' construct is used in a worksharing construct it may cause hanging of the software in case if reduction clause is used. Patch fixes this problem by avoiding extra reduction processing for branches that were canceled. llvm-svn: 287227 --- clang/lib/CodeGen/CGStmtOpenMP.cpp | 32 +++++++++---- clang/lib/CodeGen/CodeGenFunction.h | 88 ++++++++++++++++++++++++++++++++++++ clang/test/OpenMP/cancel_codegen.cpp | 7 ++- 3 files changed, 118 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp index 937295e..2f5edc8 100644 --- a/clang/lib/CodeGen/CGStmtOpenMP.cpp +++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp @@ -1781,9 +1781,11 @@ void CodeGenFunction::EmitOMPOuterLoop(bool DynamicOrOrdered, bool IsMonotonic, EmitBlock(LoopExit.getBlock()); // Tell the runtime we are done. - if (!DynamicOrOrdered) - RT.emitForStaticFinish(*this, S.getLocEnd()); - + auto &&CodeGen = [DynamicOrOrdered, &S](CodeGenFunction &CGF) { + if (!DynamicOrOrdered) + CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd()); + }; + OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen); } void CodeGenFunction::EmitOMPForOuterLoop( @@ -1899,6 +1901,8 @@ void CodeGenFunction::EmitOMPDistributeParallelForDirective( *this, OMPD_distribute_parallel_for, [&S](CodeGenFunction &CGF, PrePostActionTy &) { OMPLoopScope PreInitScope(CGF, S); + OMPCancelStackRAII CancelRegion(CGF, OMPD_distribute_parallel_for, + /*HasCancel=*/false); CGF.EmitStmt( cast(S.getAssociatedStmt())->getCapturedStmt()); }); @@ -2123,7 +2127,10 @@ bool CodeGenFunction::EmitOMPWorksharingLoop(const OMPLoopDirective &S) { [](CodeGenFunction &) {}); EmitBlock(LoopExit.getBlock()); // Tell the runtime we are done. - RT.emitForStaticFinish(*this, S.getLocStart()); + auto &&CodeGen = [&S](CodeGenFunction &CGF) { + CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd()); + }; + OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen); } else { const bool IsMonotonic = Ordered || ScheduleKind.Schedule == OMPC_SCHEDULE_static || @@ -2173,6 +2180,7 @@ void CodeGenFunction::EmitOMPForDirective(const OMPForDirective &S) { bool HasLastprivates = false; auto &&CodeGen = [&S, &HasLastprivates](CodeGenFunction &CGF, PrePostActionTy &) { + OMPCancelStackRAII CancelRegion(CGF, OMPD_for, S.hasCancel()); HasLastprivates = CGF.EmitOMPWorksharingLoop(S); }; { @@ -2313,7 +2321,10 @@ void CodeGenFunction::EmitSections(const OMPExecutableDirective &S) { CGF.EmitOMPInnerLoop(S, /*RequiresCleanup=*/false, &Cond, &Inc, BodyGen, [](CodeGenFunction &) {}); // Tell the runtime we are done. - CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocStart()); + auto &&CodeGen = [&S](CodeGenFunction &CGF) { + CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd()); + }; + CGF.OMPCancelStack.emitExit(CGF, S.getDirectiveKind(), CodeGen); CGF.EmitOMPReductionClauseFinal(S); // Emit post-update of the reduction variables if IsLastIter != 0. emitPostUpdateForReductionClause( @@ -2335,6 +2346,7 @@ void CodeGenFunction::EmitSections(const OMPExecutableDirective &S) { HasCancel = OSD->hasCancel(); else if (auto *OPSD = dyn_cast(&S)) HasCancel = OPSD->hasCancel(); + OMPCancelStackRAII CancelRegion(*this, S.getDirectiveKind(), HasCancel); CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_sections, CodeGen, HasCancel); // Emit barrier for lastprivates only if 'sections' directive has 'nowait' @@ -2438,6 +2450,7 @@ void CodeGenFunction::EmitOMPParallelForDirective( // Emit directive as a combined directive that consists of two implicit // directives: 'parallel' with 'for' directive. auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &) { + OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel()); CGF.EmitOMPWorksharingLoop(S); }; emitCommonOMPParallelDirective(*this, S, OMPD_for, CodeGen); @@ -3435,11 +3448,14 @@ void CodeGenFunction::EmitOMPCancelDirective(const OMPCancelDirective &S) { CodeGenFunction::JumpDest CodeGenFunction::getOMPCancelDestination(OpenMPDirectiveKind Kind) { - if (Kind == OMPD_parallel || Kind == OMPD_task) + if (Kind == OMPD_parallel || Kind == OMPD_task || + Kind == OMPD_target_parallel) return ReturnBlock; assert(Kind == OMPD_for || Kind == OMPD_section || Kind == OMPD_sections || - Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for); - return BreakContinueStack.back().BreakBlock; + Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for || + Kind == OMPD_distribute_parallel_for || + Kind == OMPD_target_parallel_for); + return OMPCancelStack.getExitBlock(); } void CodeGenFunction::EmitOMPUseDevicePtrClause( diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index e5ca9bc..b53287d 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -982,6 +982,94 @@ private: }; SmallVector BreakContinueStack; + /// Handles cancellation exit points in OpenMP-related constructs. + class OpenMPCancelExitStack { + /// Tracks cancellation exit point and join point for cancel-related exit + /// and normal exit. + struct CancelExit { + CancelExit() = default; + CancelExit(OpenMPDirectiveKind Kind, JumpDest ExitBlock, + JumpDest ContBlock) + : Kind(Kind), ExitBlock(ExitBlock), ContBlock(ContBlock) {} + OpenMPDirectiveKind Kind = OMPD_unknown; + /// true if the exit block has been emitted already by the special + /// emitExit() call, false if the default codegen is used. + bool HasBeenEmitted = false; + JumpDest ExitBlock; + JumpDest ContBlock; + }; + + SmallVector Stack; + + public: + OpenMPCancelExitStack() : Stack(1) {} + ~OpenMPCancelExitStack() = default; + /// Fetches the exit block for the current OpenMP construct. + JumpDest getExitBlock() const { return Stack.back().ExitBlock; } + /// Emits exit block with special codegen procedure specific for the related + /// OpenMP construct + emits code for normal construct cleanup. + void emitExit(CodeGenFunction &CGF, OpenMPDirectiveKind Kind, + const llvm::function_ref &CodeGen) { + if (Stack.back().Kind == Kind && getExitBlock().isValid()) { + assert(CGF.getOMPCancelDestination(Kind).isValid()); + assert(CGF.HaveInsertPoint()); + assert(!Stack.back().HasBeenEmitted); + auto IP = CGF.Builder.saveAndClearIP(); + CGF.EmitBlock(Stack.back().ExitBlock.getBlock()); + CodeGen(CGF); + CGF.EmitBranchThroughCleanup(Stack.back().ContBlock); + CGF.Builder.restoreIP(IP); + Stack.back().HasBeenEmitted = true; + } + CodeGen(CGF); + } + /// Enter the cancel supporting \a Kind construct. + /// \param Kind OpenMP directive that supports cancel constructs. + /// \param HasCancel true, if the construct has inner cancel directive, + /// false otherwise. + void enter(CodeGenFunction &CGF, OpenMPDirectiveKind Kind, bool HasCancel) { + Stack.push_back({Kind, + HasCancel ? CGF.getJumpDestInCurrentScope("cancel.exit") + : JumpDest(), + HasCancel ? CGF.getJumpDestInCurrentScope("cancel.cont") + : JumpDest()}); + } + /// Emits default exit point for the cancel construct (if the special one + /// has not be used) + join point for cancel/normal exits. + void exit(CodeGenFunction &CGF) { + if (getExitBlock().isValid()) { + assert(CGF.getOMPCancelDestination(Stack.back().Kind).isValid()); + bool HaveIP = CGF.HaveInsertPoint(); + if (!Stack.back().HasBeenEmitted) { + if (HaveIP) + CGF.EmitBranchThroughCleanup(Stack.back().ContBlock); + CGF.EmitBlock(Stack.back().ExitBlock.getBlock()); + CGF.EmitBranchThroughCleanup(Stack.back().ContBlock); + } + CGF.EmitBlock(Stack.back().ContBlock.getBlock()); + if (!HaveIP) { + CGF.Builder.CreateUnreachable(); + CGF.Builder.ClearInsertionPoint(); + } + } + Stack.pop_back(); + } + }; + OpenMPCancelExitStack OMPCancelStack; + + /// Controls insertion of cancellation exit blocks in worksharing constructs. + class OMPCancelStackRAII { + CodeGenFunction &CGF; + + public: + OMPCancelStackRAII(CodeGenFunction &CGF, OpenMPDirectiveKind Kind, + bool HasCancel) + : CGF(CGF) { + CGF.OMPCancelStack.enter(CGF, Kind, HasCancel); + } + ~OMPCancelStackRAII() { CGF.OMPCancelStack.exit(CGF); } + }; + CodeGenPGO PGO; /// Calculate branch weights appropriate for PGO data diff --git a/clang/test/OpenMP/cancel_codegen.cpp b/clang/test/OpenMP/cancel_codegen.cpp index fb0a4dd..5a3b96b 100644 --- a/clang/test/OpenMP/cancel_codegen.cpp +++ b/clang/test/OpenMP/cancel_codegen.cpp @@ -90,9 +90,11 @@ for (int i = 0; i < argc; ++i) { } } // CHECK: call void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call( -#pragma omp parallel for +int r = 0; +#pragma omp parallel for reduction(+: r) for (int i = 0; i < argc; ++i) { #pragma omp cancel for + r += i; } // CHECK: call void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call( return argc; @@ -163,6 +165,9 @@ for (int i = 0; i < argc; ++i) { // CHECK: [[CONTINUE]] // CHECK: br label // CHECK: call void @__kmpc_for_static_fini( +// CHECK: call i32 @__kmpc_reduce_nowait( +// CHECK: call void @__kmpc_end_reduce_nowait( +// CHECK: call void @__kmpc_for_static_fini( // CHECK: ret void #endif -- 2.7.4