https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/137193
>From ba6d59cdb2bf906a60b7e13448af730bd2019140 Mon Sep 17 00:00:00 2001 From: Tom Eccles <tom.ecc...@arm.com> Date: Tue, 8 Apr 2025 17:21:15 +0000 Subject: [PATCH 1/4] [mlir][OpenMP] Convert omp.cancel parallel to LLVMIR Support for other constructs will follow in subsequent PRs. --- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 69 +++++++++++++++- mlir/test/Target/LLVMIR/openmp-cancel.mlir | 82 +++++++++++++++++++ mlir/test/Target/LLVMIR/openmp-todo.mlir | 48 +++++++++-- 3 files changed, 191 insertions(+), 8 deletions(-) create mode 100644 mlir/test/Target/LLVMIR/openmp-cancel.mlir diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index 52aa1fbfab2c1..6185a433a8199 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -158,6 +158,12 @@ static LogicalResult checkImplementationStatus(Operation &op) { if (op.getBare()) result = todo("ompx_bare"); }; + auto checkCancelDirective = [&todo](auto op, LogicalResult &result) { + omp::ClauseCancellationConstructType cancelledDirective = + op.getCancelDirective(); + if (cancelledDirective != omp::ClauseCancellationConstructType::Parallel) + result = todo("cancel directive"); + }; auto checkDepend = [&todo](auto op, LogicalResult &result) { if (!op.getDependVars().empty() || op.getDependKinds()) result = todo("depend"); @@ -248,6 +254,7 @@ static LogicalResult checkImplementationStatus(Operation &op) { LogicalResult result = success(); llvm::TypeSwitch<Operation &>(op) + .Case([&](omp::CancelOp op) { checkCancelDirective(op, result); }) .Case([&](omp::DistributeOp op) { checkAllocate(op, result); checkDistSchedule(op, result); @@ -1580,6 +1587,21 @@ cleanupPrivateVars(llvm::IRBuilderBase &builder, return success(); } +/// Returns true if the construct contains omp.cancel or omp.cancellation_point +static bool constructIsCancellable(Operation *op) { + // omp.cancel must be "closely nested" so it will be visible and not inside of + // funcion calls. This is enforced by the verifier. + bool containsCancel = false; + op->walk([&containsCancel](Operation *child) { + if (mlir::isa<omp::CancelOp>(child)) { + containsCancel = true; + return WalkResult::interrupt(); + } + return WalkResult::advance(); + }); + return containsCancel; +} + static LogicalResult convertOmpSections(Operation &opInst, llvm::IRBuilderBase &builder, LLVM::ModuleTranslation &moduleTranslation) { @@ -2524,8 +2546,7 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder, auto pbKind = llvm::omp::OMP_PROC_BIND_default; if (auto bind = opInst.getProcBindKind()) pbKind = getProcBindKind(*bind); - // TODO: Is the Parallel construct cancellable? - bool isCancellable = false; + bool isCancellable = constructIsCancellable(opInst); llvm::OpenMPIRBuilder::InsertPointTy allocaIP = findAllocaInsertPoint(builder, moduleTranslation); @@ -2991,6 +3012,47 @@ convertOmpAtomicCapture(omp::AtomicCaptureOp atomicCaptureOp, return success(); } +static llvm::omp::Directive convertCancellationConstructType( + omp::ClauseCancellationConstructType directive) { + switch (directive) { + case omp::ClauseCancellationConstructType::Loop: + return llvm::omp::Directive::OMPD_for; + case omp::ClauseCancellationConstructType::Parallel: + return llvm::omp::Directive::OMPD_parallel; + case omp::ClauseCancellationConstructType::Sections: + return llvm::omp::Directive::OMPD_sections; + case omp::ClauseCancellationConstructType::Taskgroup: + return llvm::omp::Directive::OMPD_taskgroup; + } +} + +static LogicalResult +convertOmpCancel(omp::CancelOp op, llvm::IRBuilderBase &builder, + LLVM::ModuleTranslation &moduleTranslation) { + llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder); + llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder(); + + if (failed(checkImplementationStatus(*op.getOperation()))) + return failure(); + + llvm::Value *ifCond = nullptr; + if (Value ifVar = op.getIfExpr()) + ifCond = moduleTranslation.lookupValue(ifVar); + + llvm::omp::Directive cancelledDirective = + convertCancellationConstructType(op.getCancelDirective()); + + llvm::OpenMPIRBuilder::InsertPointOrErrorTy afterIP = + ompBuilder->createCancel(ompLoc, ifCond, cancelledDirective); + + if (failed(handleError(afterIP, *op.getOperation()))) + return failure(); + + builder.restoreIP(afterIP.get()); + + return success(); +} + /// Converts an OpenMP Threadprivate operation into LLVM IR using /// OpenMPIRBuilder. static LogicalResult @@ -5421,6 +5483,9 @@ convertHostOrTargetOperation(Operation *op, llvm::IRBuilderBase &builder, .Case([&](omp::AtomicCaptureOp op) { return convertOmpAtomicCapture(op, builder, moduleTranslation); }) + .Case([&](omp::CancelOp op) { + return convertOmpCancel(op, builder, moduleTranslation); + }) .Case([&](omp::SectionsOp) { return convertOmpSections(*op, builder, moduleTranslation); }) diff --git a/mlir/test/Target/LLVMIR/openmp-cancel.mlir b/mlir/test/Target/LLVMIR/openmp-cancel.mlir new file mode 100644 index 0000000000000..1f67d6ceb34af --- /dev/null +++ b/mlir/test/Target/LLVMIR/openmp-cancel.mlir @@ -0,0 +1,82 @@ +// RUN: mlir-translate --mlir-to-llvmir %s | FileCheck %s + +llvm.func @cancel_parallel() { + omp.parallel { + omp.cancel cancellation_construct_type(parallel) + omp.terminator + } + llvm.return +} +// CHECK-LABEL: define internal void @cancel_parallel..omp_par +// CHECK: omp.par.entry: +// CHECK: %[[VAL_5:.*]] = alloca i32, align 4 +// CHECK: %[[VAL_6:.*]] = load i32, ptr %[[VAL_7:.*]], align 4 +// CHECK: store i32 %[[VAL_6]], ptr %[[VAL_5]], align 4 +// CHECK: %[[VAL_8:.*]] = load i32, ptr %[[VAL_5]], align 4 +// CHECK: br label %[[VAL_9:.*]] +// CHECK: omp.region.after_alloca: ; preds = %[[VAL_10:.*]] +// CHECK: br label %[[VAL_11:.*]] +// CHECK: omp.par.region: ; preds = %[[VAL_9]] +// CHECK: br label %[[VAL_12:.*]] +// CHECK: omp.par.region1: ; preds = %[[VAL_11]] +// CHECK: %[[VAL_13:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: %[[VAL_14:.*]] = call i32 @__kmpc_cancel(ptr @1, i32 %[[VAL_13]], i32 1) +// CHECK: %[[VAL_15:.*]] = icmp eq i32 %[[VAL_14]], 0 +// CHECK: br i1 %[[VAL_15]], label %[[VAL_16:.*]], label %[[VAL_17:.*]] +// CHECK: omp.par.region1.cncl: ; preds = %[[VAL_12]] +// CHECK: %[[VAL_18:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: %[[VAL_19:.*]] = call i32 @__kmpc_cancel_barrier(ptr @2, i32 %[[VAL_18]]) +// CHECK: br label %[[VAL_20:.*]] +// CHECK: omp.par.region1.split: ; preds = %[[VAL_12]] +// CHECK: br label %[[VAL_21:.*]] +// CHECK: omp.region.cont: ; preds = %[[VAL_16]] +// CHECK: br label %[[VAL_22:.*]] +// CHECK: omp.par.pre_finalize: ; preds = %[[VAL_21]] +// CHECK: br label %[[VAL_20]] +// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_22]], %[[VAL_17]] +// CHECK: ret void + +llvm.func @cancel_parallel_if(%arg0 : i1) { + omp.parallel { + omp.cancel cancellation_construct_type(parallel) if(%arg0) + omp.terminator + } + llvm.return +} +// CHECK-LABEL: define internal void @cancel_parallel_if..omp_par +// CHECK: omp.par.entry: +// CHECK: %[[VAL_9:.*]] = getelementptr { ptr }, ptr %[[VAL_10:.*]], i32 0, i32 0 +// CHECK: %[[VAL_11:.*]] = load ptr, ptr %[[VAL_9]], align 8 +// CHECK: %[[VAL_12:.*]] = alloca i32, align 4 +// CHECK: %[[VAL_13:.*]] = load i32, ptr %[[VAL_14:.*]], align 4 +// CHECK: store i32 %[[VAL_13]], ptr %[[VAL_12]], align 4 +// CHECK: %[[VAL_15:.*]] = load i32, ptr %[[VAL_12]], align 4 +// CHECK: %[[VAL_16:.*]] = load i1, ptr %[[VAL_11]], align 1 +// CHECK: br label %[[VAL_17:.*]] +// CHECK: omp.region.after_alloca: ; preds = %[[VAL_18:.*]] +// CHECK: br label %[[VAL_19:.*]] +// CHECK: omp.par.region: ; preds = %[[VAL_17]] +// CHECK: br label %[[VAL_20:.*]] +// CHECK: omp.par.region1: ; preds = %[[VAL_19]] +// CHECK: br i1 %[[VAL_16]], label %[[VAL_21:.*]], label %[[VAL_22:.*]] +// CHECK: 3: ; preds = %[[VAL_20]] +// CHECK: br label %[[VAL_23:.*]] +// CHECK: 4: ; preds = %[[VAL_22]], %[[VAL_24:.*]] +// CHECK: br label %[[VAL_25:.*]] +// CHECK: omp.region.cont: ; preds = %[[VAL_23]] +// CHECK: br label %[[VAL_26:.*]] +// CHECK: omp.par.pre_finalize: ; preds = %[[VAL_25]] +// CHECK: br label %[[VAL_27:.*]] +// CHECK: 5: ; preds = %[[VAL_20]] +// CHECK: %[[VAL_28:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: %[[VAL_29:.*]] = call i32 @__kmpc_cancel(ptr @1, i32 %[[VAL_28]], i32 1) +// CHECK: %[[VAL_30:.*]] = icmp eq i32 %[[VAL_29]], 0 +// CHECK: br i1 %[[VAL_30]], label %[[VAL_24]], label %[[VAL_31:.*]] +// CHECK: .cncl: ; preds = %[[VAL_21]] +// CHECK: %[[VAL_32:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: %[[VAL_33:.*]] = call i32 @__kmpc_cancel_barrier(ptr @2, i32 %[[VAL_32]]) +// CHECK: br label %[[VAL_27]] +// CHECK: .split: ; preds = %[[VAL_21]] +// CHECK: br label %[[VAL_23]] +// CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_31]], %[[VAL_26]] +// CHECK: ret void diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir index 7eafe396082e4..bf251ac2b7d0a 100644 --- a/mlir/test/Target/LLVMIR/openmp-todo.mlir +++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir @@ -26,12 +26,48 @@ llvm.func @atomic_hint(%v : !llvm.ptr, %x : !llvm.ptr, %expr : i32) { // ----- -llvm.func @cancel() { - // expected-error@below {{LLVM Translation failed for operation: omp.parallel}} - omp.parallel { - // expected-error@below {{not yet implemented: omp.cancel}} - // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} - omp.cancel cancellation_construct_type(parallel) +llvm.func @cancel_wsloop(%lb : i32, %ub : i32, %step: i32) { + // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}} + omp.wsloop { + // expected-error@below {{LLVM Translation failed for operation: omp.loop_nest}} + omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) { + // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} + omp.cancel cancellation_construct_type(loop) + omp.yield + } + } + llvm.return +} + +// ----- + +llvm.func @cancel_sections() { + // expected-error@below {{LLVM Translation failed for operation: omp.sections}} + omp.sections { + omp.section { + // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} + omp.cancel cancellation_construct_type(sections) + omp.terminator + } + omp.terminator + } + llvm.return +} + +// ----- + +llvm.func @cancel_taskgroup() { + // expected-error@below {{LLVM Translation failed for operation: omp.taskgroup}} + omp.taskgroup { + // expected-error@below {{LLVM Translation failed for operation: omp.task}} + omp.task { + // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} + omp.cancel cancellation_construct_type(taskgroup) + omp.terminator + } omp.terminator } llvm.return >From 4fb5d42996e3d9958a1e00df303694576e01775b Mon Sep 17 00:00:00 2001 From: Tom Eccles <tom.ecc...@arm.com> Date: Fri, 25 Apr 2025 14:08:10 +0000 Subject: [PATCH 2/4] Review comments --- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 18 ++++++++---------- mlir/test/Target/LLVMIR/openmp-todo.mlir | 6 +++--- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index 6185a433a8199..b7f63d1b00d37 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -162,7 +162,7 @@ static LogicalResult checkImplementationStatus(Operation &op) { omp::ClauseCancellationConstructType cancelledDirective = op.getCancelDirective(); if (cancelledDirective != omp::ClauseCancellationConstructType::Parallel) - result = todo("cancel directive"); + result = todo("cancel directive construct type not yet supported"); }; auto checkDepend = [&todo](auto op, LogicalResult &result) { if (!op.getDependVars().empty() || op.getDependKinds()) @@ -1591,15 +1591,13 @@ cleanupPrivateVars(llvm::IRBuilderBase &builder, static bool constructIsCancellable(Operation *op) { // omp.cancel must be "closely nested" so it will be visible and not inside of // funcion calls. This is enforced by the verifier. - bool containsCancel = false; - op->walk([&containsCancel](Operation *child) { - if (mlir::isa<omp::CancelOp>(child)) { - containsCancel = true; - return WalkResult::interrupt(); - } - return WalkResult::advance(); - }); - return containsCancel; + return op + ->walk([](Operation *child) { + if (mlir::isa<omp::CancelOp>(child)) + return WalkResult::interrupt(); + return WalkResult::advance(); + }) + .wasInterrupted(); } static LogicalResult diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir index bf251ac2b7d0a..a408a5a9c8682 100644 --- a/mlir/test/Target/LLVMIR/openmp-todo.mlir +++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir @@ -31,7 +31,7 @@ llvm.func @cancel_wsloop(%lb : i32, %ub : i32, %step: i32) { omp.wsloop { // expected-error@below {{LLVM Translation failed for operation: omp.loop_nest}} omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) { - // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}} // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} omp.cancel cancellation_construct_type(loop) omp.yield @@ -46,7 +46,7 @@ llvm.func @cancel_sections() { // expected-error@below {{LLVM Translation failed for operation: omp.sections}} omp.sections { omp.section { - // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}} // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} omp.cancel cancellation_construct_type(sections) omp.terminator @@ -63,7 +63,7 @@ llvm.func @cancel_taskgroup() { omp.taskgroup { // expected-error@below {{LLVM Translation failed for operation: omp.task}} omp.task { - // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}} + // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}} // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} omp.cancel cancellation_construct_type(taskgroup) omp.terminator >From f3b8eebb37320ab748682f6a091e52e64aeec0d7 Mon Sep 17 00:00:00 2001 From: Tom Eccles <tom.ecc...@arm.com> Date: Thu, 10 Apr 2025 11:43:18 +0000 Subject: [PATCH 3/4] [mlir][OpenMP] Convert omp.cancel sections to LLVMIR This is quite ugly but it is the best I could think of. The old FiniCBWrapper was way too brittle depending upon the exact block structure inside of the section, and could be confused by any control flow in the section (e.g. an if clause on cancel). The wording in the comment and variable names didn't seem to match where it was actually branching too as well. Clang's (non-OpenMPIRBuilder) lowering for cancel inside of sections branches to a block containing __kmpc_for_static_fini. This was hard to achieve here because sometimes the FiniCBWrapper has to run before the worksharing loop finalization has been crated. To get around this ordering issue I created a dummy branch to a dummy block, which is then fixed later once all of the information is available. --- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 27 ++++--- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 6 +- mlir/test/Target/LLVMIR/openmp-cancel.mlir | 76 +++++++++++++++++++ mlir/test/Target/LLVMIR/openmp-todo.mlir | 16 ---- 4 files changed, 97 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index be05f01c94603..3f19088e6c73d 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -2172,6 +2172,9 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createSections( if (!updateToLocation(Loc)) return Loc.IP; + // FiniCBWrapper needs to create a branch to the loop finalization block, but + // this has not been created yet at some times when this callback runs. + SmallVector<BranchInst *> CancellationBranches; auto FiniCBWrapper = [&](InsertPointTy IP) { if (IP.getBlock()->end() != IP.getPoint()) return FiniCB(IP); @@ -2179,16 +2182,9 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createSections( // will fail because that function requires the Finalization Basic Block to // have a terminator, which is already removed by EmitOMPRegionBody. // IP is currently at cancelation block. - // We need to backtrack to the condition block to fetch - // the exit block and create a branch from cancelation - // to exit block. - IRBuilder<>::InsertPointGuard IPG(Builder); - Builder.restoreIP(IP); - auto *CaseBB = IP.getBlock()->getSinglePredecessor(); - auto *CondBB = CaseBB->getSinglePredecessor()->getSinglePredecessor(); - auto *ExitBB = CondBB->getTerminator()->getSuccessor(1); - Instruction *I = Builder.CreateBr(ExitBB); - IP = InsertPointTy(I->getParent(), I->getIterator()); + BranchInst *DummyBranch = Builder.CreateBr(IP.getBlock()); + IP = InsertPointTy(DummyBranch->getParent(), DummyBranch->getIterator()); + CancellationBranches.push_back(DummyBranch); return FiniCB(IP); }; @@ -2251,6 +2247,9 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createSections( return WsloopIP.takeError(); InsertPointTy AfterIP = *WsloopIP; + BasicBlock *LoopFini = AfterIP.getBlock()->getSinglePredecessor(); + assert(LoopFini && "Bad structure of static workshare loop finalization"); + // Apply the finalization callback in LoopAfterBB auto FiniInfo = FinalizationStack.pop_back_val(); assert(FiniInfo.DK == OMPD_sections && @@ -2264,6 +2263,14 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createSections( AfterIP = {FiniBB, FiniBB->begin()}; } + // Now we can fix the dummy branch to point to the right place + if (!CancellationBranches.empty()) { + for (BranchInst *DummyBranch : CancellationBranches) { + assert(DummyBranch->getNumSuccessors() == 1); + DummyBranch->setSuccessor(0, LoopFini); + } + } + return AfterIP; } diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index b7f63d1b00d37..e172b003176a5 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -161,7 +161,8 @@ static LogicalResult checkImplementationStatus(Operation &op) { auto checkCancelDirective = [&todo](auto op, LogicalResult &result) { omp::ClauseCancellationConstructType cancelledDirective = op.getCancelDirective(); - if (cancelledDirective != omp::ClauseCancellationConstructType::Parallel) + if (cancelledDirective != omp::ClauseCancellationConstructType::Parallel && + cancelledDirective != omp::ClauseCancellationConstructType::Sections) result = todo("cancel directive construct type not yet supported"); }; auto checkDepend = [&todo](auto op, LogicalResult &result) { @@ -1688,10 +1689,11 @@ convertOmpSections(Operation &opInst, llvm::IRBuilderBase &builder, auto finiCB = [&](InsertPointTy codeGenIP) { return llvm::Error::success(); }; allocaIP = findAllocaInsertPoint(builder, moduleTranslation); + bool isCancellable = constructIsCancellable(sectionsOp); llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder); llvm::OpenMPIRBuilder::InsertPointOrErrorTy afterIP = moduleTranslation.getOpenMPBuilder()->createSections( - ompLoc, allocaIP, sectionCBs, privCB, finiCB, false, + ompLoc, allocaIP, sectionCBs, privCB, finiCB, isCancellable, sectionsOp.getNowait()); if (failed(handleError(afterIP, opInst))) diff --git a/mlir/test/Target/LLVMIR/openmp-cancel.mlir b/mlir/test/Target/LLVMIR/openmp-cancel.mlir index 1f67d6ceb34af..fca16b936fc85 100644 --- a/mlir/test/Target/LLVMIR/openmp-cancel.mlir +++ b/mlir/test/Target/LLVMIR/openmp-cancel.mlir @@ -80,3 +80,79 @@ llvm.func @cancel_parallel_if(%arg0 : i1) { // CHECK: br label %[[VAL_23]] // CHECK: omp.par.exit.exitStub: ; preds = %[[VAL_31]], %[[VAL_26]] // CHECK: ret void + +llvm.func @cancel_sections_if(%cond : i1) { + omp.sections { + omp.section { + omp.cancel cancellation_construct_type(sections) if(%cond) + omp.terminator + } + omp.terminator + } + llvm.return +} +// CHECK-LABEL: define void @cancel_sections_if +// CHECK: %[[VAL_0:.*]] = alloca i32, align 4 +// CHECK: %[[VAL_1:.*]] = alloca i32, align 4 +// CHECK: %[[VAL_2:.*]] = alloca i32, align 4 +// CHECK: %[[VAL_3:.*]] = alloca i32, align 4 +// CHECK: br label %[[VAL_4:.*]] +// CHECK: entry: ; preds = %[[VAL_5:.*]] +// CHECK: br label %[[VAL_6:.*]] +// CHECK: omp_section_loop.preheader: ; preds = %[[VAL_4]] +// CHECK: store i32 0, ptr %[[VAL_1]], align 4 +// CHECK: store i32 0, ptr %[[VAL_2]], align 4 +// CHECK: store i32 1, ptr %[[VAL_3]], align 4 +// CHECK: %[[VAL_7:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: call void @__kmpc_for_static_init_4u(ptr @1, i32 %[[VAL_7]], i32 34, ptr %[[VAL_0]], ptr %[[VAL_1]], ptr %[[VAL_2]], ptr %[[VAL_3]], i32 1, i32 0) +// CHECK: %[[VAL_8:.*]] = load i32, ptr %[[VAL_1]], align 4 +// CHECK: %[[VAL_9:.*]] = load i32, ptr %[[VAL_2]], align 4 +// CHECK: %[[VAL_10:.*]] = sub i32 %[[VAL_9]], %[[VAL_8]] +// CHECK: %[[VAL_11:.*]] = add i32 %[[VAL_10]], 1 +// CHECK: br label %[[VAL_12:.*]] +// CHECK: omp_section_loop.header: ; preds = %[[VAL_13:.*]], %[[VAL_6]] +// CHECK: %[[VAL_14:.*]] = phi i32 [ 0, %[[VAL_6]] ], [ %[[VAL_15:.*]], %[[VAL_13]] ] +// CHECK: br label %[[VAL_16:.*]] +// CHECK: omp_section_loop.cond: ; preds = %[[VAL_12]] +// CHECK: %[[VAL_17:.*]] = icmp ult i32 %[[VAL_14]], %[[VAL_11]] +// CHECK: br i1 %[[VAL_17]], label %[[VAL_18:.*]], label %[[VAL_19:.*]] +// CHECK: omp_section_loop.body: ; preds = %[[VAL_16]] +// CHECK: %[[VAL_20:.*]] = add i32 %[[VAL_14]], %[[VAL_8]] +// CHECK: %[[VAL_21:.*]] = mul i32 %[[VAL_20]], 1 +// CHECK: %[[VAL_22:.*]] = add i32 %[[VAL_21]], 0 +// CHECK: switch i32 %[[VAL_22]], label %[[VAL_23:.*]] [ +// CHECK: i32 0, label %[[VAL_24:.*]] +// CHECK: ] +// CHECK: omp_section_loop.body.case: ; preds = %[[VAL_18]] +// CHECK: br label %[[VAL_25:.*]] +// CHECK: omp.section.region: ; preds = %[[VAL_24]] +// CHECK: br i1 %[[VAL_26:.*]], label %[[VAL_27:.*]], label %[[VAL_28:.*]] +// CHECK: 9: ; preds = %[[VAL_25]] +// CHECK: %[[VAL_29:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: %[[VAL_30:.*]] = call i32 @__kmpc_cancel(ptr @1, i32 %[[VAL_29]], i32 3) +// CHECK: %[[VAL_31:.*]] = icmp eq i32 %[[VAL_30]], 0 +// CHECK: br i1 %[[VAL_31]], label %[[VAL_32:.*]], label %[[VAL_33:.*]] +// CHECK: .split: ; preds = %[[VAL_27]] +// CHECK: br label %[[VAL_34:.*]] +// CHECK: 12: ; preds = %[[VAL_25]] +// CHECK: br label %[[VAL_34]] +// CHECK: 13: ; preds = %[[VAL_28]], %[[VAL_32]] +// CHECK: br label %[[VAL_35:.*]] +// CHECK: omp.region.cont: ; preds = %[[VAL_34]] +// CHECK: br label %[[VAL_23]] +// CHECK: omp_section_loop.body.sections.after: ; preds = %[[VAL_35]], %[[VAL_18]] +// CHECK: br label %[[VAL_13]] +// CHECK: omp_section_loop.inc: ; preds = %[[VAL_23]] +// CHECK: %[[VAL_15]] = add nuw i32 %[[VAL_14]], 1 +// CHECK: br label %[[VAL_12]] +// CHECK: omp_section_loop.exit: ; preds = %[[VAL_33]], %[[VAL_16]] +// CHECK: call void @__kmpc_for_static_fini(ptr @1, i32 %[[VAL_7]]) +// CHECK: %[[VAL_36:.*]] = call i32 @__kmpc_global_thread_num(ptr @1) +// CHECK: call void @__kmpc_barrier(ptr @2, i32 %[[VAL_36]]) +// CHECK: br label %[[VAL_37:.*]] +// CHECK: omp_section_loop.after: ; preds = %[[VAL_19]] +// CHECK: br label %[[VAL_38:.*]] +// CHECK: omp_section_loop.aftersections.fini: ; preds = %[[VAL_37]] +// CHECK: ret void +// CHECK: .cncl: ; preds = %[[VAL_27]] +// CHECK: br label %[[VAL_19]] diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir index a408a5a9c8682..0cc96deacd954 100644 --- a/mlir/test/Target/LLVMIR/openmp-todo.mlir +++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir @@ -42,22 +42,6 @@ llvm.func @cancel_wsloop(%lb : i32, %ub : i32, %step: i32) { // ----- -llvm.func @cancel_sections() { - // expected-error@below {{LLVM Translation failed for operation: omp.sections}} - omp.sections { - omp.section { - // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}} - // expected-error@below {{LLVM Translation failed for operation: omp.cancel}} - omp.cancel cancellation_construct_type(sections) - omp.terminator - } - omp.terminator - } - llvm.return -} - -// ----- - llvm.func @cancel_taskgroup() { // expected-error@below {{LLVM Translation failed for operation: omp.taskgroup}} omp.taskgroup { >From 60592cf957e89ed849677374dbd6133fbcd9a771 Mon Sep 17 00:00:00 2001 From: Tom Eccles <tom.ecc...@arm.com> Date: Mon, 28 Apr 2025 14:58:38 +0000 Subject: [PATCH 4/4] Don't use clang's callback to hack the cancellation branch This conflicted with my hack doing the same thing: leading to a use-after-free of the old terminator. I think it is more helpful to do this in OpenMPIRBuilder than replicating the hack in callbacks in both clang and mlir. --- clang/lib/CodeGen/CGStmtOpenMP.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp index 156f64bb5f508..cd3ded088ee27 100644 --- a/clang/lib/CodeGen/CGStmtOpenMP.cpp +++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp @@ -4345,8 +4345,9 @@ void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) { using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy; using BodyGenCallbackTy = llvm::OpenMPIRBuilder::StorableBodyGenCallbackTy; - auto FiniCB = [this](InsertPointTy IP) { - OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP); + auto FiniCB = [](InsertPointTy IP) { + // Don't FinalizeOMPRegion because this is done inside of OMPIRBuilder for + // sections. return llvm::Error::success(); }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits