[Mlir-commits] [flang] [mlir] [mlir][omp] Improve canonloop/iv naming (PR #159773)
Michael Kruse
llvmlistbot at llvm.org
Thu Oct 2 05:54:59 PDT 2025
https://github.com/Meinersbur updated https://github.com/llvm/llvm-project/pull/159773
>From b3919715ebe223b39dd789dcd471a864666d7008 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Fri, 19 Sep 2025 14:43:48 +0200
Subject: [PATCH 01/11] Improve canonloop/iv naming
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 237 +++++++++++++-----
.../Dialect/OpenMP/cli-canonical_loop.mlir | 127 ++++++++--
.../Dialect/OpenMP/cli-unroll-heuristic.mlir | 28 +--
3 files changed, 292 insertions(+), 100 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 3d70e28ed23ab..cf549a6bb50b4 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -77,6 +77,178 @@ struct LLVMPointerPointerLikeModel
};
} // namespace
+/// Generate a name of a canonical loop nest of the format
+/// `<prefix>(_s<num>_r<num>)*` that describes its nesting inside parent
+/// operations (`_r<num>`) and that operation's region (`_s<num>`). The region
+/// number is omitted if the parent operation has just one region. If a loop
+/// nest just consists of canonical loops nested inside each other, also uses
+/// `d<num>` where <num> is the nesting depth of the loop.
+static std::string generateLoopNestingName(StringRef prefix,
+ CanonicalLoopOp op) {
+ struct Component {
+ // An region argument of an operation
+ Operation *parentOp;
+ size_t regionInOpIdx;
+ bool isOnlyRegionInOp;
+ bool skipRegion;
+
+ // An operation somewhere in a parent region
+ Operation *thisOp;
+ Region *parentRegion;
+ size_t opInRegionIdx;
+ bool isOnlyOpInRegion;
+ bool skipOp;
+ int depth = -1;
+ };
+ SmallVector<Component> components;
+
+ // Gather a list of parent regions and operations, and the position within
+ // their parent
+ Operation *o = op.getOperation();
+ while (o) {
+ if (o->hasTrait<mlir::OpTrait::IsIsolatedFromAbove>())
+ break;
+
+ // Operation within a region
+ Region *r = o->getParentRegion();
+ if (!r)
+ break;
+
+ llvm::ReversePostOrderTraversal<Block *> traversal(&r->getBlocks().front());
+ size_t idx = 0;
+ bool found = false;
+ size_t sequentialIdx = -1;
+ bool isOnlyLoop = true;
+ for (Block *b : traversal) {
+ for (Operation &op : *b) {
+ if (&op == o && !found) {
+ sequentialIdx = idx;
+ found = true;
+ }
+ if (op.getNumRegions()) {
+ idx += 1;
+ if (idx > 1)
+ isOnlyLoop = false;
+ }
+ if (found && !isOnlyLoop)
+ break;
+ }
+ }
+
+ Component &comp = components.emplace_back();
+ comp.thisOp = o;
+ comp.parentRegion = r;
+ comp.opInRegionIdx = sequentialIdx;
+ comp.isOnlyOpInRegion = isOnlyLoop;
+
+ // Region argument of an operation
+ Operation *parent = r->getParentOp();
+
+ comp.parentOp = parent;
+ comp.regionInOpIdx = 0;
+ comp.isOnlyRegionInOp = true;
+ if (parent && parent->getRegions().size() > 1) {
+ auto getRegionIndex = [](Operation *o, Region *r) {
+ for (auto [idx, region] : llvm::enumerate(o->getRegions())) {
+ if (®ion == r)
+ return idx;
+ }
+ llvm_unreachable("Region not child of its parent operation");
+ };
+ comp.regionInOpIdx = getRegionIndex(parent, r);
+ comp.isOnlyRegionInOp = false;
+ }
+
+ if (!parent)
+ break;
+
+ // next parent
+ o = parent;
+ }
+
+ // Reorder components from outermost to innermost
+ std::reverse(components.begin(), components.end());
+
+ // Determine whether a component is not needed
+ for (auto &c : components) {
+ c.skipRegion = c.isOnlyRegionInOp;
+ c.skipOp = c.isOnlyOpInRegion && !isa<CanonicalLoopOp>(c.thisOp);
+ }
+
+ // Find runs of perfect nests and merge them into a single component
+ int curNestRoot = 0;
+ int curNestDepth = 1;
+ auto mergeLoopNest = [&](int innermost) {
+ auto outermost = curNestRoot;
+
+ // Don't do enything if it does not consist of at least 2 loops
+ if (outermost < innermost) {
+ for (auto i : llvm::seq<int>(outermost + 1, innermost)) {
+ components[i].skipOp = true;
+ }
+ components[innermost].depth = curNestDepth;
+ }
+
+ // Start new root
+ curNestRoot = innermost + 1;
+ curNestDepth = 1;
+ };
+ for (auto &&[i, c] : llvm::enumerate(components)) {
+ if (i <= curNestRoot)
+ continue;
+
+ // Check whether this region can be included
+ if (!c.skipRegion) {
+ mergeLoopNest(i);
+ continue;
+ }
+
+ if (c.skipOp)
+ continue;
+
+ if (!c.isOnlyOpInRegion) {
+ mergeLoopNest(i);
+ continue;
+ }
+
+ curNestDepth += 1;
+ }
+
+ // Finalize innermost loop nest
+ mergeLoopNest(components.size() - 1);
+
+ // Outermost loop does not need a suffix if it has no sibling
+ for (auto &c : components) {
+ if (c.skipOp)
+ continue;
+ if (c.isOnlyOpInRegion)
+ c.skipOp = true;
+ break;
+ }
+
+ // Compile name
+ SmallString<64> Name{prefix};
+ for (auto &c : components) {
+ auto addComponent = [&Name](char letter, int64_t idx) {
+ Name += '_';
+ Name += letter;
+ Name += idx;
+ };
+
+ if (!c.skipRegion)
+ addComponent('r', c.regionInOpIdx);
+
+ if (!c.skipOp) {
+ if (c.depth >= 0)
+ addComponent('d', c.depth - 1);
+ else
+ addComponent('s', c.opInRegionIdx);
+ }
+ }
+
+ return Name.str().str();
+}
+
void OpenMPDialect::initialize() {
addOperations<
#define GET_OP_LIST
@@ -3141,67 +3313,7 @@ void NewCliOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
cliName =
TypeSwitch<Operation *, std::string>(gen->getOwner())
.Case([&](CanonicalLoopOp op) {
- // Find the canonical loop nesting: For each ancestor add a
- // "+_r<idx>" suffix (in reverse order)
- SmallVector<std::string> components;
- Operation *o = op.getOperation();
- while (o) {
- if (o->hasTrait<mlir::OpTrait::IsIsolatedFromAbove>())
- break;
-
- Region *r = o->getParentRegion();
- if (!r)
- break;
-
- auto getSequentialIndex = [](Region *r, Operation *o) {
- llvm::ReversePostOrderTraversal<Block *> traversal(
- &r->getBlocks().front());
- size_t idx = 0;
- for (Block *b : traversal) {
- for (Operation &op : *b) {
- if (&op == o)
- return idx;
- // Only consider operations that are containers as
- // possible children
- if (!op.getRegions().empty())
- idx += 1;
- }
- }
- llvm_unreachable("Operation not part of the region");
- };
- size_t sequentialIdx = getSequentialIndex(r, o);
- components.push_back(("s" + Twine(sequentialIdx)).str());
-
- Operation *parent = r->getParentOp();
- if (!parent)
- break;
-
- // If the operation has more than one region, also count in
- // which of the regions
- if (parent->getRegions().size() > 1) {
- auto getRegionIndex = [](Operation *o, Region *r) {
- for (auto [idx, region] :
- llvm::enumerate(o->getRegions())) {
- if (®ion == r)
- return idx;
- }
- llvm_unreachable("Region not child its parent operation");
- };
- size_t regionIdx = getRegionIndex(parent, r);
- components.push_back(("r" + Twine(regionIdx)).str());
- }
-
- // next parent
- o = parent;
- }
-
- SmallString<64> Name("canonloop");
- for (const std::string &s : reverse(components)) {
- Name += '_';
- Name += s;
- }
-
- return Name;
+ return generateLoopNestingName("canonloop", op);
})
.Case([&](UnrollHeuristicOp op) -> std::string {
llvm_unreachable("heuristic unrolling does not generate a loop");
@@ -3292,7 +3404,8 @@ void CanonicalLoopOp::getAsmBlockNames(OpAsmSetBlockNameFn setNameFn) {
void CanonicalLoopOp::getAsmBlockArgumentNames(Region ®ion,
OpAsmSetValueNameFn setNameFn) {
- setNameFn(region.getArgument(0), "iv");
+ std::string ivName = generateLoopNestingName("iv", *this);
+ setNameFn(region.getArgument(0), ivName);
}
void CanonicalLoopOp::print(OpAsmPrinter &p) {
diff --git a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
index adadb8bbac49d..874e3922805ec 100644
--- a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
+++ b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-opt %s | FileCheck %s
-// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+// RUN: mlir-opt %s | FileCheck %s --enable-var-scope
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s --enable-var-scope
// CHECK-LABEL: @omp_canonloop_raw(
@@ -24,10 +24,10 @@ func.func @omp_canonloop_raw(%tc : i32) -> () {
func.func @omp_canonloop_sequential_raw(%tc : i32) -> () {
// CHECK-NEXT: %canonloop_s0 = omp.new_cli
%canonloop_s0 = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv_s0 : i32 in range(%[[tc]]) {
"omp.canonical_loop" (%tc, %canonloop_s0) ({
^bb_first(%iv_first: i32):
- // CHECK-NEXT: = llvm.add %iv, %iv : i32
+ // CHECK-NEXT: = llvm.add %iv_s0, %iv_s0 : i32
%newval = llvm.add %iv_first, %iv_first : i32
// CHECK-NEXT: omp.terminator
omp.terminator
@@ -36,7 +36,7 @@ func.func @omp_canonloop_sequential_raw(%tc : i32) -> () {
// CHECK-NEXT: %canonloop_s1 = omp.new_cli
%canonloop_s1 = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s1) %iv : i32 in range(%[[tc]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s1) %iv_s1 : i32 in range(%[[tc]]) {
"omp.canonical_loop" (%tc, %canonloop_s1) ({
^bb_second(%iv_second: i32):
// CHECK: omp.terminator
@@ -52,17 +52,17 @@ func.func @omp_canonloop_sequential_raw(%tc : i32) -> () {
// CHECK-LABEL: @omp_nested_canonloop_raw(
// CHECK-SAME: %[[tc_outer:.+]]: i32, %[[tc_inner:.+]]: i32)
func.func @omp_nested_canonloop_raw(%tc_outer : i32, %tc_inner : i32) -> () {
- // CHECK-NEXT: %canonloop_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop = omp.new_cli
%outer = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: %canonloop_s0_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop_d1 = omp.new_cli
%inner = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc_outer]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc_outer]]) {
"omp.canonical_loop" (%tc_outer, %outer) ({
^bb_outer(%iv_outer: i32):
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0_s0) %iv_0 : i32 in range(%[[tc_inner]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_d1) %iv_d1 : i32 in range(%[[tc_inner]]) {
"omp.canonical_loop" (%tc_inner, %inner) ({
^bb_inner(%iv_inner: i32):
- // CHECK-NEXT: = llvm.add %iv, %iv_0 : i32
+ // CHECK-NEXT: = llvm.add %iv, %iv_d1 : i32
%newval = llvm.add %iv_outer, %iv_inner: i32
// CHECK-NEXT: omp.terminator
omp.terminator
@@ -108,16 +108,24 @@ func.func @omp_canonloop_constant_pretty() -> () {
func.func @omp_canonloop_sequential_pretty(%tc : i32) -> () {
// CHECK-NEXT: %canonloop_s0 = omp.new_cli
%canonloop_s0 = omp.new_cli
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
- omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv_s0 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s0) %iv_s0 : i32 in range(%tc) {
// CHECK-NEXT: omp.terminator
omp.terminator
}
// CHECK: %canonloop_s1 = omp.new_cli
%canonloop_s1 = omp.new_cli
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s1) %iv : i32 in range(%[[tc]]) {
- omp.canonical_loop(%canonloop_s1) %iv_0 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s1) %iv_s1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s1) %iv_s1 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ }
+
+ // CHECK: %canonloop_s2 = omp.new_cli
+ %canonloop_s2 = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s2) %iv_s2 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s2) %iv_s2 : i32 in range(%tc) {
// CHECK-NEXT: omp.terminator
omp.terminator
}
@@ -126,17 +134,17 @@ func.func @omp_canonloop_sequential_pretty(%tc : i32) -> () {
}
-// CHECK-LABEL: @omp_canonloop_nested_pretty(
+// CHECK-LABEL: @omp_canonloop_2d_nested_pretty(
// CHECK-SAME: %[[tc:.+]]: i32)
-func.func @omp_canonloop_nested_pretty(%tc : i32) -> () {
- // CHECK-NEXT: %canonloop_s0 = omp.new_cli
- %canonloop_s0 = omp.new_cli
- // CHECK-NEXT: %canonloop_s0_s0 = omp.new_cli
- %canonloop_s0_s0 = omp.new_cli
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
- omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%tc) {
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0_s0) %iv_0 : i32 in range(%[[tc]]) {
- omp.canonical_loop(%canonloop_s0_s0) %iv_0 : i32 in range(%tc) {
+func.func @omp_canonloop_2d_nested_pretty(%tc : i32) -> () {
+ // CHECK-NEXT: %canonloop = omp.new_cli
+ %canonloop = omp.new_cli
+ // CHECK-NEXT: %canonloop_d1 = omp.new_cli
+ %canonloop_d1 = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop) %iv : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_d1) %iv_d1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_d1) %iv_d1 : i32 in range(%tc) {
// CHECK: omp.terminator
omp.terminator
}
@@ -147,6 +155,77 @@ func.func @omp_canonloop_nested_pretty(%tc : i32) -> () {
}
+// CHECK-LABEL: @omp_canonloop_3d_nested_pretty(
+// CHECK-SAME: %[[tc:.+]]: i32)
+func.func @omp_canonloop_3d_nested_pretty(%tc : i32) -> () {
+ // CHECK: %canonloop = omp.new_cli
+ %canonloop = omp.new_cli
+ // CHECK: %canonloop_d1 = omp.new_cli
+ %canonloop_d1 = omp.new_cli
+ // CHECK: %canonloop_d2 = omp.new_cli
+ %canonloop_d2 = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop) %iv : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_d1) %iv_d1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_d1) %iv_1d : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_d2) %iv_d2 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_d2) %iv_d2 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ // CHECK-NEXT: }
+ }
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ // CHECK-NEXT: }
+ }
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ }
+
+ return
+}
+
+
+// CHECK-LABEL: @omp_canonloop_sequential_nested_pretty(
+// CHECK-SAME: %[[tc:.+]]: i32)
+func.func @omp_canonloop_sequential_nested_pretty(%tc : i32) -> () {
+ // CHECK-NEXT: %canonloop_s0 = omp.new_cli
+ %canonloop_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop_s0_d1 = omp.new_cli
+ %canonloop_s0_d1 = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv_s0 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s0) %iv_s0 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s0_d1) %iv_s0_d1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s0_d1) %iv_s0_d1 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ // CHECK-NEXT: }
+ }
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ // CHECK-NEXT: }
+ }
+
+ // CHECK-NEXT: %canonloop_s1 = omp.new_cli
+ %canonloop_s1 = omp.new_cli
+ // CHECK-NEXT: %canonloop_s1_d1 = omp.new_cli
+ %canonloop_s1_d1 = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s1) %iv_s1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s1) %iv_s1 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_s1_d1) %iv_s1_d1 : i32 in range(%[[tc]]) {
+ omp.canonical_loop(%canonloop_s1_d1) %iv_s1d1 : i32 in range(%tc) {
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ // CHECK-NEXT: }
+ }
+ // CHECK-NEXT: omp.terminator
+ omp.terminator
+ }
+
+ return
+}
+
+
// CHECK-LABEL: @omp_newcli_unused(
// CHECK-SAME: )
func.func @omp_newcli_unused() -> () {
diff --git a/mlir/test/Dialect/OpenMP/cli-unroll-heuristic.mlir b/mlir/test/Dialect/OpenMP/cli-unroll-heuristic.mlir
index cda7d0b500166..16884f4245e76 100644
--- a/mlir/test/Dialect/OpenMP/cli-unroll-heuristic.mlir
+++ b/mlir/test/Dialect/OpenMP/cli-unroll-heuristic.mlir
@@ -1,18 +1,18 @@
-// RUN: mlir-opt %s | FileCheck %s
-// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+// RUN: mlir-opt %s | FileCheck %s --enable-var-scope
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s --enable-var-scope
// CHECK-LABEL: @omp_unroll_heuristic_raw(
// CHECK-SAME: %[[tc:.+]]: i32) {
func.func @omp_unroll_heuristic_raw(%tc : i32) -> () {
- // CHECK-NEXT: %canonloop_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop = omp.new_cli
%canonloop = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc]]) {
"omp.canonical_loop" (%tc, %canonloop) ({
^bb0(%iv: i32):
omp.terminator
}) : (i32, !omp.cli) -> ()
- // CHECK: omp.unroll_heuristic(%canonloop_s0)
+ // CHECK: omp.unroll_heuristic(%canonloop)
"omp.unroll_heuristic" (%canonloop) : (!omp.cli) -> ()
return
}
@@ -22,12 +22,12 @@ func.func @omp_unroll_heuristic_raw(%tc : i32) -> () {
// CHECK-SAME: %[[tc:.+]]: i32) {
func.func @omp_unroll_heuristic_pretty(%tc : i32) -> () {
// CHECK-NEXT: %[[CANONLOOP:.+]] = omp.new_cli
- %canonloop = "omp.new_cli" () : () -> (!omp.cli)
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
+ %canonloop = omp.new_cli
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc]]) {
omp.canonical_loop(%canonloop) %iv : i32 in range(%tc) {
omp.terminator
}
- // CHECK: omp.unroll_heuristic(%canonloop_s0)
+ // CHECK: omp.unroll_heuristic(%canonloop)
omp.unroll_heuristic(%canonloop)
return
}
@@ -36,13 +36,13 @@ func.func @omp_unroll_heuristic_pretty(%tc : i32) -> () {
// CHECK-LABEL: @omp_unroll_heuristic_nested_pretty(
// CHECK-SAME: %[[tc:.+]]: i32) {
func.func @omp_unroll_heuristic_nested_pretty(%tc : i32) -> () {
- // CHECK-NEXT: %canonloop_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop = omp.new_cli
%cli_outer = omp.new_cli
- // CHECK-NEXT: %canonloop_s0_s0 = omp.new_cli
+ // CHECK-NEXT: %canonloop_d1 = omp.new_cli
%cli_inner = omp.new_cli
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0) %iv : i32 in range(%[[tc]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop) %iv : i32 in range(%[[tc]]) {
omp.canonical_loop(%cli_outer) %iv_outer : i32 in range(%tc) {
- // CHECK-NEXT: omp.canonical_loop(%canonloop_s0_s0) %iv_0 : i32 in range(%[[tc]]) {
+ // CHECK-NEXT: omp.canonical_loop(%canonloop_d1) %iv_d1 : i32 in range(%[[tc]]) {
omp.canonical_loop(%cli_inner) %iv_inner : i32 in range(%tc) {
// CHECK: omp.terminator
omp.terminator
@@ -51,9 +51,9 @@ func.func @omp_unroll_heuristic_nested_pretty(%tc : i32) -> () {
omp.terminator
}
- // CHECK: omp.unroll_heuristic(%canonloop_s0)
+ // CHECK: omp.unroll_heuristic(%canonloop)
omp.unroll_heuristic(%cli_outer)
- // CHECK-NEXT: omp.unroll_heuristic(%canonloop_s0_s0)
+ // CHECK-NEXT: omp.unroll_heuristic(%canonloop_d1)
omp.unroll_heuristic(%cli_inner)
return
}
>From ce66eec648f6415c199d6115f3be4d188eee59ba Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 23 Sep 2025 12:19:41 +0200
Subject: [PATCH 02/11] Avoid compiler warning
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index cf549a6bb50b4..1674891410194 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -170,22 +170,21 @@ static std::string generateLoopNestingName(StringRef prefix,
std::reverse(components.begin(), components.end());
// Determine whether a component is not needed
- for (auto &c : components) {
+ for (Component &c : components) {
c.skipRegion = c.isOnlyRegionInOp;
c.skipOp = c.isOnlyOpInRegion && !isa<CanonicalLoopOp>(c.thisOp);
}
// Find runs of perfect nests and merge them into a single component
- int curNestRoot = 0;
- int curNestDepth = 1;
- auto mergeLoopNest = [&](int innermost) {
- auto outermost = curNestRoot;
+ size_t curNestRoot = 0;
+ size_t curNestDepth = 1;
+ auto mergeLoopNest = [&](size_t innermost) {
+ size_t outermost = curNestRoot;
// Don't do enything if it does not consist of at least 2 loops
if (outermost < innermost) {
- for (auto i : llvm::seq<int>(outermost + 1, innermost)) {
+ for (auto i : llvm::seq<int>(outermost + 1, innermost))
components[i].skipOp = true;
- }
components[innermost].depth = curNestDepth;
}
>From 3a141d6729e26a7eab821b86eee240ab4bfa322f Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 23 Sep 2025 12:59:14 +0200
Subject: [PATCH 03/11] Add perfect-nest and rectangular loop nest tests
---
flang/lib/Semantics/resolve-directives.cpp | 146 ++++++++++++++++++++-
flang/test/Semantics/OpenMP/do08.f90 | 1 +
flang/test/Semantics/OpenMP/do13.f90 | 1 +
flang/test/Semantics/OpenMP/do22.f90 | 73 +++++++++++
4 files changed, 215 insertions(+), 6 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/do22.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2d1bec9968593..5f2c9f676099c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -149,6 +149,9 @@ template <typename T> class DirectiveAttributeVisitor {
dataSharingAttributeObjects_.clear();
}
bool HasDataSharingAttributeObject(const Symbol &);
+ std::tuple<const parser::Name *, const parser::ScalarExpr *,
+ const parser::ScalarExpr *, const parser::ScalarExpr *>
+ GetLoopBounds(const parser::DoConstruct &);
const parser::Name *GetLoopIndex(const parser::DoConstruct &);
const parser::DoConstruct *GetDoConstructIf(
const parser::ExecutionPartConstruct &);
@@ -933,6 +936,13 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
privateDataSharingAttributeObjects_.clear();
}
+ /// Check that loops in the loop nest are perfectly nested, as well that lower
+ /// bound, upper bound, and step expressions do not use the iv
+ /// of a surrounding loop of the associated loops nest.
+ /// We do not support non-perfectly nested loops not non-rectangular loops yet
+ /// (both introduced in OpenMP 5.0)
+ void CheckPerfectNestAndRectangularLoop(const parser::OpenMPLoopConstruct &x);
+
// Predetermined DSA rules
void PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const parser::OpenMPLoopConstruct &);
@@ -1009,14 +1019,15 @@ bool DirectiveAttributeVisitor<T>::HasDataSharingAttributeObject(
}
template <typename T>
-const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
- const parser::DoConstruct &x) {
+std::tuple<const parser::Name *, const parser::ScalarExpr *,
+ const parser::ScalarExpr *, const parser::ScalarExpr *>
+DirectiveAttributeVisitor<T>::GetLoopBounds(const parser::DoConstruct &x) {
using Bounds = parser::LoopControl::Bounds;
if (x.GetLoopControl()) {
if (const Bounds * b{std::get_if<Bounds>(&x.GetLoopControl()->u)}) {
- return &b->name.thing;
- } else {
- return nullptr;
+ auto &&step = b->step;
+ return {&b->name.thing, &b->lower, &b->upper,
+ step.has_value() ? &step.value() : nullptr};
}
} else {
context_
@@ -1024,8 +1035,15 @@ const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
"Loop control is not present in the DO LOOP"_err_en_US)
.Attach(GetContext().directiveSource,
"associated with the enclosing LOOP construct"_en_US);
- return nullptr;
}
+ return {nullptr, nullptr, nullptr, nullptr};
+}
+
+template <typename T>
+const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
+ const parser::DoConstruct &x) {
+ auto &&[iv, lb, ub, step] = GetLoopBounds(x);
+ return iv;
}
template <typename T>
@@ -1957,6 +1975,7 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
}
}
}
+ CheckPerfectNestAndRectangularLoop(x);
PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
ordCollapseLevel = GetNumAffectedLoopsFromLoopConstruct(x) + 1;
return true;
@@ -2152,6 +2171,121 @@ void OmpAttributeVisitor::CollectNumAffectedLoopsFromClauses(
}
}
+void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
+ const parser::OpenMPLoopConstruct
+ &x) { // GetAssociatedLoopLevelFromClauses(clauseList);
+ auto &&dirContext = GetContext();
+ std::int64_t dirDepth{dirContext.associatedLoopLevel};
+ if (dirDepth <= 0)
+ return;
+
+ Symbol::Flag ivDSA;
+ if (!llvm::omp::allSimdSet.test(GetContext().directive)) {
+ ivDSA = Symbol::Flag::OmpPrivate;
+ } else if (dirDepth == 1) {
+ ivDSA = Symbol::Flag::OmpLinear;
+ } else {
+ ivDSA = Symbol::Flag::OmpLastPrivate;
+ }
+
+ auto checkExprHasSymbols = [&](llvm::SmallVector<Symbol *> &ivs,
+ const parser::ScalarExpr *bound) {
+ if (ivs.empty())
+ return;
+
+ if (auto boundExpr{semantics::AnalyzeExpr(context_, *bound)}) {
+ semantics::UnorderedSymbolSet boundSyms =
+ evaluate::CollectSymbols(*boundExpr);
+ for (auto iv : ivs) {
+ if (boundSyms.count(*iv) != 0) {
+ // TODO: Point to occurence of iv in boundExpr, directiveSource as a
+ // note
+ context_.Say(dirContext.directiveSource,
+ "Trip count must be computable and invariant"_err_en_US);
+ }
+ }
+ }
+ };
+
+ // Skip over loop transformation directives
+ const parser::OpenMPLoopConstruct *innerMostLoop = &x;
+ const parser::NestedConstruct *innerMostNest = nullptr;
+ while (auto &optLoopCons{
+ std::get<std::optional<parser::NestedConstruct>>(innerMostLoop->t)}) {
+ innerMostNest = &(optLoopCons.value());
+ if (const auto *innerLoop{
+ std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
+ innerMostNest)}) {
+ innerMostLoop = &(innerLoop->value());
+ } else
+ break;
+ }
+
+ if (!innerMostNest)
+ return;
+ const auto &outer{std::get_if<parser::DoConstruct>(innerMostNest)};
+ if (!outer)
+ return;
+
+ llvm::SmallVector<Symbol *> ivs;
+ int curLevel = 0;
+ const parser::DoConstruct *loop{outer};
+ while (true) {
+ auto [iv, lb, ub, step] = GetLoopBounds(*loop);
+
+ if (lb)
+ checkExprHasSymbols(ivs, lb);
+ if (ub)
+ checkExprHasSymbols(ivs, ub);
+ if (step)
+ checkExprHasSymbols(ivs, step);
+ if (iv) {
+ if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())})
+ ivs.push_back(symbol);
+ }
+
+ // Stop after processing all affected loops
+ if (curLevel + 1 >= dirDepth)
+ break;
+
+ // Recurse into nested loop
+ const auto &block{std::get<parser::Block>(loop->t)};
+ if (block.empty()) {
+ // Insufficient number of nested loops already reported by
+ // CheckAssocLoopLevel()
+ break;
+ }
+
+ loop = GetDoConstructIf(block.front());
+ if (!loop) {
+ // Insufficient number of nested loops already reported by
+ // CheckAssocLoopLevel()
+ break;
+ }
+
+ auto checkPerfectNest = [&, this]() {
+ auto blockSize = block.size();
+ if (blockSize <= 1)
+ return;
+
+ if (parser::Unwrap<parser::ContinueStmt>(x))
+ blockSize -= 1;
+
+ if (blockSize <= 1)
+ return;
+
+ // Non-perfectly nested loop
+ // TODO: Point to non-DO statement, directiveSource as a note
+ context_.Say(dirContext.directiveSource,
+ "Canonical loop nest must be perfectly nested."_err_en_US);
+ };
+
+ checkPerfectNest();
+
+ ++curLevel;
+ }
+}
+
// 2.15.1.1 Data-sharing Attribute Rules - Predetermined
// - The loop iteration variable(s) in the associated do-loop(s) of a do,
// parallel do, taskloop, or distribute construct is (are) private.
diff --git a/flang/test/Semantics/OpenMP/do08.f90 b/flang/test/Semantics/OpenMP/do08.f90
index 5143dff0dd315..bb3c1d0cd3855 100644
--- a/flang/test/Semantics/OpenMP/do08.f90
+++ b/flang/test/Semantics/OpenMP/do08.f90
@@ -61,6 +61,7 @@ program omp
!$omp end do
+ !ERROR: Canonical loop nest must be perfectly nested.
!ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
!$omp do collapse(3)
do 60 i=2,200,2
diff --git a/flang/test/Semantics/OpenMP/do13.f90 b/flang/test/Semantics/OpenMP/do13.f90
index 6e9d1dddade4c..8f7844f4136f9 100644
--- a/flang/test/Semantics/OpenMP/do13.f90
+++ b/flang/test/Semantics/OpenMP/do13.f90
@@ -59,6 +59,7 @@ program omp
!$omp end do
+ !ERROR: Canonical loop nest must be perfectly nested.
!ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
!$omp do collapse(3)
do 60 i=1,10
diff --git a/flang/test/Semantics/OpenMP/do22.f90 b/flang/test/Semantics/OpenMP/do22.f90
new file mode 100644
index 0000000000000..9d96d3af54e5c
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/do22.f90
@@ -0,0 +1,73 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Check for existence of loop following a DO directive
+
+subroutine do_imperfectly_nested_before
+ integer i, j
+
+ !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
+ !$omp do collapse(2)
+ do i = 1, 10
+ print *, i
+ do j = 1, 10
+ print *, i, j
+ end do
+ end do
+ !$omp end do
+end subroutine
+
+
+subroutine do_imperfectly_nested_behind
+ integer i, j
+
+ !ERROR: Canonical loop nest must be perfectly nested.
+ !$omp do collapse(2)
+ do i = 1, 10
+ do j = 1, 10
+ print *, i, j
+ end do
+ print *, i
+ end do
+ !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_lb
+ integer i, j
+
+ !ERROR: Trip count must be computable and invariant
+ !$omp do collapse(2)
+ do i = 1, 10
+ do j = i, 10
+ print *, i, j
+ end do
+ end do
+ !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_ub
+ integer i, j
+
+ !ERROR: Trip count must be computable and invariant
+ !$omp do collapse(2)
+ do i = 1, 10
+ do j = 0, i
+ print *, i, j
+ end do
+ end do
+ !$omp end do
+end subroutine
+
+
+subroutine do_nonrectangular_step
+ integer i, j
+
+ !ERROR: Trip count must be computable and invariant
+ !$omp do collapse(2)
+ do i = 1, 10
+ do j = 1, 10, i
+ print *, i, j
+ end do
+ end do
+ !$omp end do
+end subroutine
>From 493442453cbac2366aa89abde361f7badaad4948 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 23 Sep 2025 16:39:44 +0200
Subject: [PATCH 04/11] Fix symbol resolution
---
flang/lib/Semantics/resolve-directives.cpp | 48 ++++++++++------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 5f2c9f676099c..a0109324e546c 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1975,7 +1975,10 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
}
}
}
+
+ // Must be done before iv privatization
CheckPerfectNestAndRectangularLoop(x);
+
PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
ordCollapseLevel = GetNumAffectedLoopsFromLoopConstruct(x) + 1;
return true;
@@ -2172,37 +2175,29 @@ void OmpAttributeVisitor::CollectNumAffectedLoopsFromClauses(
}
void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
- const parser::OpenMPLoopConstruct
- &x) { // GetAssociatedLoopLevelFromClauses(clauseList);
- auto &&dirContext = GetContext();
+ const parser::OpenMPLoopConstruct &x) {
+ auto &dirContext = GetContext();
std::int64_t dirDepth{dirContext.associatedLoopLevel};
if (dirDepth <= 0)
return;
- Symbol::Flag ivDSA;
- if (!llvm::omp::allSimdSet.test(GetContext().directive)) {
- ivDSA = Symbol::Flag::OmpPrivate;
- } else if (dirDepth == 1) {
- ivDSA = Symbol::Flag::OmpLinear;
- } else {
- ivDSA = Symbol::Flag::OmpLastPrivate;
- }
-
auto checkExprHasSymbols = [&](llvm::SmallVector<Symbol *> &ivs,
const parser::ScalarExpr *bound) {
if (ivs.empty())
return;
-
- if (auto boundExpr{semantics::AnalyzeExpr(context_, *bound)}) {
- semantics::UnorderedSymbolSet boundSyms =
- evaluate::CollectSymbols(*boundExpr);
- for (auto iv : ivs) {
- if (boundSyms.count(*iv) != 0) {
- // TODO: Point to occurence of iv in boundExpr, directiveSource as a
- // note
- context_.Say(dirContext.directiveSource,
- "Trip count must be computable and invariant"_err_en_US);
- }
+ auto boundExpr{semantics::AnalyzeExpr(context_, *bound)};
+ if (!boundExpr)
+ return;
+ semantics::UnorderedSymbolSet boundSyms =
+ evaluate::CollectSymbols(*boundExpr);
+ if (boundSyms.empty())
+ return;
+ for (Symbol *iv : ivs) {
+ if (boundSyms.count(*iv) != 0) {
+ // TODO: Point to occurence of iv in boundExpr, directiveSource as a
+ // note
+ context_.Say(dirContext.directiveSource,
+ "Trip count must be computable and invariant"_err_en_US);
}
}
};
@@ -2217,8 +2212,9 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
innerMostNest)}) {
innerMostLoop = &(innerLoop->value());
- } else
+ } else {
break;
+ }
}
if (!innerMostNest)
@@ -2228,7 +2224,7 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
return;
llvm::SmallVector<Symbol *> ivs;
- int curLevel = 0;
+ int curLevel{0};
const parser::DoConstruct *loop{outer};
while (true) {
auto [iv, lb, ub, step] = GetLoopBounds(*loop);
@@ -2240,7 +2236,7 @@ void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
if (step)
checkExprHasSymbols(ivs, step);
if (iv) {
- if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())})
+ if (auto *symbol{currScope().FindSymbol(iv->source)})
ivs.push_back(symbol);
}
>From b1c6e22532c6103c1cc9153e2f0036a438b482ad Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Mon, 29 Sep 2025 14:27:45 +0200
Subject: [PATCH 05/11] Handle IsolatedFromAbove individually
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 15 ++++---
.../Dialect/OpenMP/cli-canonical_loop.mlir | 44 +++++++++++++++++++
2 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 1674891410194..44ef943b63335 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -106,9 +106,6 @@ static std::string generateLoopNestingName(StringRef prefix,
// their parent
Operation *o = op.getOperation();
while (o) {
- if (o->hasTrait<mlir::OpTrait::IsIsolatedFromAbove>())
- break;
-
// Operation within a region
Region *r = o->getParentRegion();
if (!r)
@@ -147,7 +144,14 @@ static std::string generateLoopNestingName(StringRef prefix,
comp.parentOp = parent;
comp.regionInOpIdx = 0;
comp.isOnlyRegionInOp = true;
- if (parent && parent->getRegions().size() > 1) {
+
+ // Need to disambiguate between different region arguments? The
+ // IsolatedFromAbove trait of the parent operation implies that each
+ // individual region argument has its own separate namespace.
+ if (!parent || parent->hasTrait<mlir::OpTrait::IsIsolatedFromAbove>())
+ break;
+
+ if (parent->getRegions().size() > 1) {
auto getRegionIndex = [](Operation *o, Region *r) {
for (auto [idx, region] : llvm::enumerate(o->getRegions())) {
if (®ion == r)
@@ -159,9 +163,6 @@ static std::string generateLoopNestingName(StringRef prefix,
comp.isOnlyRegionInOp = false;
}
- if (!parent)
- break;
-
// next parent
o = parent;
}
diff --git a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
index 874e3922805ec..c77253cea94fe 100644
--- a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
+++ b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
@@ -234,3 +234,47 @@ func.func @omp_newcli_unused() -> () {
// CHECK-NEXT: return
return
}
+
+
+// CHECK-LABEL: @omp_canonloop_multiregion_isolatedfromabove(
+func.func @omp_canonloop_multiregion_isolatedfromabove() -> () {
+ omp.private {type = firstprivate} @x.privatizer : !llvm.ptr init {
+ ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+ %c42_i32 = arith.constant 42: i32
+ // CHECK: omp.canonical_loop %iv : i32 in range(%c42_i32) {
+ omp.canonical_loop %iv1 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ // CHECK: omp.yield
+ omp.yield(%arg0 : !llvm.ptr)
+ } copy {
+ ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+ %c42_i32 = arith.constant 42: i32
+ // CHECK: omp.canonical_loop %iv : i32 in range(%c42_i32) {
+ omp.canonical_loop %iv : i32 in range(%c42_i32) {
+ // CHECK: omp.canonical_loop %iv_d1 : i32 in range(%c42_i32) {
+ omp.canonical_loop %iv_d1 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ omp.terminator
+ }
+ // CHECK: omp.yield
+ omp.yield(%arg0 : !llvm.ptr)
+ } dealloc {
+ ^bb0(%arg0: !llvm.ptr):
+ %c42_i32 = arith.constant 42: i32
+ // CHECK: omp.canonical_loop %iv_s0 : i32 in range(%c42_i32) {
+ omp.canonical_loop %iv_s0 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ // CHECK: omp.canonical_loop %iv_s1 : i32 in range(%c42_i32) {
+ omp.canonical_loop %iv_s1 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ // CHECK: omp.yield
+ omp.yield
+ }
+
+ // CHECK: return
+ return
+}
>From a2b0388ebf3a4c21dec7ba294e01d817d728d6ce Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Mon, 29 Sep 2025 14:42:21 +0200
Subject: [PATCH 06/11] clarify function comment
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 44ef943b63335..fbb1161097a05 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -78,11 +78,19 @@ struct LLVMPointerPointerLikeModel
} // namespace
/// Generate a name of a canonical loop nest of the format
-/// `<prefix>(_s<num>_r<num>)*` that describes its nesting inside parent
-/// operations (`_r<num>`) and that operation's region (`_s<num>`). The region
-/// number is omitted if the parent operation has just one region. If a loop
-/// nest just consists of canonical loops nested inside each other, also uses
-/// `d<num>` where <num> is the nesting depth of the loop.
+/// `<prefix>(_r<idx>_s<idx>)*`. Hereby, `_r<idx>` identifies the region
+/// argument index of an operation that has multiple regions, if the operation
+/// has multiple regions.
+/// `_s<idx>` identifies the position of an operation within a region, where
+/// only operations that may potentially contain loops (i.e. have region
+/// arguments) are counted. Again, it is omitted if there is only one such
+/// operation in a region. If there are canonical loops nested inside each
+/// other, also may also use the format `_d<num>` where <num> is the nesting
+/// depth of the loop.
+///
+/// The generated name is a best-effort to make canonical loop unique within an
+/// SSA namespace. This also means that regions with IsolatedFromAbove property
+/// do not consider any parents or siblings.
static std::string generateLoopNestingName(StringRef prefix,
CanonicalLoopOp op) {
struct Component {
>From 3564371e1d2bc4ffcaedbb54038d69630c7a24f7 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Mon, 29 Sep 2025 14:47:58 +0200
Subject: [PATCH 07/11] isOnlyOpInRegion -> isOnlyContainerOpInRegion
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 36 ++++++++++----------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index fbb1161097a05..15453631b2f59 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -82,7 +82,7 @@ struct LLVMPointerPointerLikeModel
/// argument index of an operation that has multiple regions, if the operation
/// has multiple regions.
/// `_s<idx>` identifies the position of an operation within a region, where
-/// only operations that may potentially contain loops (i.e. have region
+/// only operations that may potentially contain loops ("container operations" i.e. have region
/// arguments) are counted. Again, it is omitted if there is only one such
/// operation in a region. If there are canonical loops nested inside each
/// other, also may also use the format `_d<num>` where <num> is the nesting
@@ -100,12 +100,12 @@ static std::string generateLoopNestingName(StringRef prefix,
bool isOnlyRegionInOp;
bool skipRegion;
- // An operation somewhere in a parent region
- Operation *thisOp;
+ // An container operation somewhere in a parent region
+ Operation *containerOp;
Region *parentRegion;
- size_t opInRegionIdx;
- bool isOnlyOpInRegion;
- bool skipOp;
+ size_t containerOpInRegionIdx;
+ bool isOnlyContainerOpInRegion;
+ bool skipContainerOp;
int depth = -1;
};
SmallVector<Component> components;
@@ -141,10 +141,10 @@ static std::string generateLoopNestingName(StringRef prefix,
}
Component &comp = components.emplace_back();
- comp.thisOp = o;
+ comp.containerOp = o;
comp.parentRegion = r;
- comp.opInRegionIdx = sequentialIdx;
- comp.isOnlyOpInRegion = isOnlyLoop;
+ comp.containerOpInRegionIdx = sequentialIdx;
+ comp.isOnlyContainerOpInRegion = isOnlyLoop;
// Region argument of an operation
Operation *parent = r->getParentOp();
@@ -181,7 +181,7 @@ static std::string generateLoopNestingName(StringRef prefix,
// Determine whether a component is not needed
for (Component &c : components) {
c.skipRegion = c.isOnlyRegionInOp;
- c.skipOp = c.isOnlyOpInRegion && !isa<CanonicalLoopOp>(c.thisOp);
+ c.skipContainerOp = c.isOnlyContainerOpInRegion && !isa<CanonicalLoopOp>(c.containerOp);
}
// Find runs of perfect nests and merge them into a single component
@@ -193,7 +193,7 @@ static std::string generateLoopNestingName(StringRef prefix,
// Don't do enything if it does not consist of at least 2 loops
if (outermost < innermost) {
for (auto i : llvm::seq<int>(outermost + 1, innermost))
- components[i].skipOp = true;
+ components[i].skipContainerOp = true;
components[innermost].depth = curNestDepth;
}
@@ -211,10 +211,10 @@ static std::string generateLoopNestingName(StringRef prefix,
continue;
}
- if (c.skipOp)
+ if (c.skipContainerOp)
continue;
- if (!c.isOnlyOpInRegion) {
+ if (!c.isOnlyContainerOpInRegion) {
mergeLoopNest(i);
continue;
}
@@ -227,10 +227,10 @@ static std::string generateLoopNestingName(StringRef prefix,
// Outermost loop does not need a suffix if it has no sibling
for (auto &c : components) {
- if (c.skipOp)
+ if (c.skipContainerOp)
continue;
- if (c.isOnlyOpInRegion)
- c.skipOp = true;
+ if (c.isOnlyContainerOpInRegion)
+ c.skipContainerOp = true;
break;
}
@@ -246,11 +246,11 @@ static std::string generateLoopNestingName(StringRef prefix,
if (!c.skipRegion)
addComponent('r', c.regionInOpIdx);
- if (!c.skipOp) {
+ if (!c.skipContainerOp) {
if (c.depth >= 0)
addComponent('d', c.depth - 1);
else
- addComponent('s', c.opInRegionIdx);
+ addComponent('s', c.containerOpInRegionIdx);
}
}
>From e41023737571e742dd5ad82304692a6f2c235ef1 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Mon, 29 Sep 2025 17:41:17 +0200
Subject: [PATCH 08/11] rework algorithm
---
flang/test/Fir/omp-cli.fir | 28 +++
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 218 ++++++++++---------
2 files changed, 149 insertions(+), 97 deletions(-)
create mode 100644 flang/test/Fir/omp-cli.fir
diff --git a/flang/test/Fir/omp-cli.fir b/flang/test/Fir/omp-cli.fir
new file mode 100644
index 0000000000000..8e5ea00b9744a
--- /dev/null
+++ b/flang/test/Fir/omp-cli.fir
@@ -0,0 +1,28 @@
+// RUN: fir-opt %s | FileCheck %s --enable-var-scope
+// RUN: fir-opt %s | fir-opt | FileCheck %s --enable-var-scope
+
+// CHECK-LABEL: @omp_canonloop_multiregion(
+func.func @omp_canonloop_multiregion(%c : i1) -> () {
+ %c42_i32 = arith.constant 42: i32
+ %canonloop1 = omp.new_cli
+ %canonloop2 = omp.new_cli
+ %canonloop3 = omp.new_cli
+ fir.if %c {
+ // CHECK: omp.canonical_loop(%canonloop_r0) %iv_r0 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop1) %iv1 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ } else {
+ // CHECK: omp.canonical_loop(%canonloop_r1_s0) %iv_r1_s0 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop2) %iv2 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ // CHECK: omp.canonical_loop(%canonloop_r1_s1) %iv_r1_s1 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop3) %iv3 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ }
+
+ // CHECK: fir.unreachable
+ fir.unreachable
+}
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 15453631b2f59..5d4039ba37b83 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -82,11 +82,11 @@ struct LLVMPointerPointerLikeModel
/// argument index of an operation that has multiple regions, if the operation
/// has multiple regions.
/// `_s<idx>` identifies the position of an operation within a region, where
-/// only operations that may potentially contain loops ("container operations" i.e. have region
-/// arguments) are counted. Again, it is omitted if there is only one such
-/// operation in a region. If there are canonical loops nested inside each
-/// other, also may also use the format `_d<num>` where <num> is the nesting
-/// depth of the loop.
+/// only operations that may potentially contain loops ("container operations"
+/// i.e. have region arguments) are counted. Again, it is omitted if there is
+/// only one such operation in a region. If there are canonical loops nested
+/// inside each other, also may also use the format `_d<num>` where <num> is the
+/// nesting depth of the loop.
///
/// The generated name is a best-effort to make canonical loop unique within an
/// SSA namespace. This also means that regions with IsolatedFromAbove property
@@ -94,20 +94,31 @@ struct LLVMPointerPointerLikeModel
static std::string generateLoopNestingName(StringRef prefix,
CanonicalLoopOp op) {
struct Component {
- // An region argument of an operation
- Operation *parentOp;
- size_t regionInOpIdx;
- bool isOnlyRegionInOp;
- bool skipRegion;
-
- // An container operation somewhere in a parent region
- Operation *containerOp;
- Region *parentRegion;
- size_t containerOpInRegionIdx;
- bool isOnlyContainerOpInRegion;
- bool skipContainerOp;
- int depth = -1;
+ bool isRegionArgOfOp;
+ bool skip = false;
+ bool isUnique = false;
+ size_t idx;
+
+ union {
+ /// isRegionArgOfOp == true: region argument of an operation
+ struct {
+ Operation *ownerOp;
+ };
+ /// isRegionArgOfOp == false: container op in a region
+ struct {
+ Operation *containerOp;
+ Region *parentRegion;
+ size_t loopDepth;
+ };
+ };
+
+ void skipIf(bool v = true) { skip = skip || v; }
};
+
+ // List of ancestors, from inner to outer.
+ // Alternates between
+ // * region argument of an operation
+ // * operation within a region
SmallVector<Component> components;
// Gather a list of parent regions and operations, and the position within
@@ -123,7 +134,7 @@ static std::string generateLoopNestingName(StringRef prefix,
size_t idx = 0;
bool found = false;
size_t sequentialIdx = -1;
- bool isOnlyLoop = true;
+ bool isOnlyContainerOp = true;
for (Block *b : traversal) {
for (Operation &op : *b) {
if (&op == o && !found) {
@@ -133,32 +144,37 @@ static std::string generateLoopNestingName(StringRef prefix,
if (op.getNumRegions()) {
idx += 1;
if (idx > 1)
- isOnlyLoop = false;
+ isOnlyContainerOp = false;
}
- if (found && !isOnlyLoop)
+ if (found && !isOnlyContainerOp)
break;
}
}
- Component &comp = components.emplace_back();
- comp.containerOp = o;
- comp.parentRegion = r;
- comp.containerOpInRegionIdx = sequentialIdx;
- comp.isOnlyContainerOpInRegion = isOnlyLoop;
+ Component &containerOpInRegion = components.emplace_back();
+ containerOpInRegion.isRegionArgOfOp = false;
+ containerOpInRegion.containerOp = o;
+ containerOpInRegion.parentRegion = r;
+ containerOpInRegion.idx = sequentialIdx;
+ containerOpInRegion.isUnique = isOnlyContainerOp;
- // Region argument of an operation
Operation *parent = r->getParentOp();
- comp.parentOp = parent;
- comp.regionInOpIdx = 0;
- comp.isOnlyRegionInOp = true;
-
- // Need to disambiguate between different region arguments? The
- // IsolatedFromAbove trait of the parent operation implies that each
- // individual region argument has its own separate namespace.
+ // Region argument of an operation
+ Component ®ionArgOfOperation = components.emplace_back();
+ regionArgOfOperation.isRegionArgOfOp = true;
+ regionArgOfOperation.ownerOp = parent;
+ regionArgOfOperation.idx = 0;
+ regionArgOfOperation.isUnique = true;
+
+ // The IsolatedFromAbove trait of the parent operation implies that each
+ // individual region argument has its own separate namespace, so no
+ // ambiguity.
if (!parent || parent->hasTrait<mlir::OpTrait::IsIsolatedFromAbove>())
break;
+ // Component only needed if operation has multiple region operands. Region
+ // arguments may be optional, but we currently do not consider this.
if (parent->getRegions().size() > 1) {
auto getRegionIndex = [](Operation *o, Region *r) {
for (auto [idx, region] : llvm::enumerate(o->getRegions())) {
@@ -167,94 +183,102 @@ static std::string generateLoopNestingName(StringRef prefix,
}
llvm_unreachable("Region not child of its parent operation");
};
- comp.regionInOpIdx = getRegionIndex(parent, r);
- comp.isOnlyRegionInOp = false;
+ regionArgOfOperation.idx = getRegionIndex(parent, r);
+ regionArgOfOperation.isUnique = false;
}
// next parent
o = parent;
}
- // Reorder components from outermost to innermost
- std::reverse(components.begin(), components.end());
-
- // Determine whether a component is not needed
- for (Component &c : components) {
- c.skipRegion = c.isOnlyRegionInOp;
- c.skipContainerOp = c.isOnlyContainerOpInRegion && !isa<CanonicalLoopOp>(c.containerOp);
- }
+ // Determine whether a region-argument component is not needed
+ for (Component &c : components)
+ c.skipIf(c.isRegionArgOfOp && c.isUnique);
- // Find runs of perfect nests and merge them into a single component
- size_t curNestRoot = 0;
- size_t curNestDepth = 1;
- auto mergeLoopNest = [&](size_t innermost) {
- size_t outermost = curNestRoot;
-
- // Don't do enything if it does not consist of at least 2 loops
- if (outermost < innermost) {
- for (auto i : llvm::seq<int>(outermost + 1, innermost))
- components[i].skipContainerOp = true;
- components[innermost].depth = curNestDepth;
- }
-
- // Start new root
- curNestRoot = innermost + 1;
- curNestDepth = 1;
- };
- for (auto &&[i, c] : llvm::enumerate(components)) {
- if (i <= curNestRoot)
+ // Find runs of nested loops and determine each loop's depth in the loop nest
+ size_t numSurroundingLoops = 0;
+ for (Component &c : llvm::reverse(components)) {
+ if (c.skip)
continue;
- // Check whether this region can be included
- if (!c.skipRegion) {
- mergeLoopNest(i);
+ // non-skipped multi-argument operands interrupt the loop nest
+ if (c.isRegionArgOfOp) {
+ numSurroundingLoops = 0;
continue;
}
- if (c.skipContainerOp)
- continue;
+ // Multiple loops in a region means each of them is the outermost loop of a
+ // new loop nest
+ if (!c.isUnique)
+ numSurroundingLoops = 0;
- if (!c.isOnlyContainerOpInRegion) {
- mergeLoopNest(i);
- continue;
- }
+ c.loopDepth = numSurroundingLoops;
- curNestDepth += 1;
+ // Next loop is surrounded by one more loop
+ if (isa<CanonicalLoopOp>(c.containerOp))
+ numSurroundingLoops += 1;
}
- // Finalize innermost loop nest
- mergeLoopNest(components.size() - 1);
-
- // Outermost loop does not need a suffix if it has no sibling
- for (auto &c : components) {
- if (c.skipContainerOp)
+ // In loop nests, skip all but the innermost loop that contains the depth
+ // number
+ bool isLoopNest = false;
+ for (Component &c : components) {
+ if (c.skip || c.isRegionArgOfOp)
continue;
- if (c.isOnlyContainerOpInRegion)
- c.skipContainerOp = true;
- break;
+
+ if (!isLoopNest && c.loopDepth >= 1) {
+ // Innermost loop of a loop nest of at least two loops
+ isLoopNest = true;
+ } else if (isLoopNest) {
+ // Non-innermost loop of a loop nest
+ c.skipIf(c.isUnique);
+
+ // If there is no surrounding loop left, this must have been the outermost
+ // loop; leave loop-nest mode for the next iteration
+ if (c.loopDepth == 0)
+ isLoopNest = false;
+ }
}
- // Compile name
- SmallString<64> Name{prefix};
- for (auto &c : components) {
- auto addComponent = [&Name](char letter, int64_t idx) {
- Name += '_';
- Name += letter;
- Name += idx;
- };
+ // Skip non-loop unambiguous regions (but they should interrupt loop nests, so
+ // we mark them as skipped only after computing loop nests)
+ for (Component &c : components)
+ c.skipIf(!c.isRegionArgOfOp && c.isUnique &&
+ !isa<CanonicalLoopOp>(c.containerOp));
+
+ // Components can be skipped if they are already disambiguated by their parent
+ // (or does not have a parent)
+ bool newRegion = true;
+ for (Component &c : llvm::reverse(components)) {
+ c.skipIf(newRegion && c.isUnique);
+
+ // non-skipped components disambiguate unique children
+ if (!c.skip)
+ newRegion = true;
+
+ // ...except canonical loops that need a suffix for each nest
+ if (!c.isRegionArgOfOp && isa<CanonicalLoopOp>(c.containerOp))
+ newRegion = false;
+ }
- if (!c.skipRegion)
- addComponent('r', c.regionInOpIdx);
+ // Compile the nesting name string
+ SmallString<64> Name{prefix};
+ llvm::raw_svector_ostream NameOS(Name);
+ for (auto &c : llvm::reverse(components)) {
+ if (c.skip)
+ continue;
- if (!c.skipContainerOp) {
- if (c.depth >= 0)
- addComponent('d', c.depth - 1);
+ if (c.isRegionArgOfOp) {
+ NameOS << "_r" << c.idx;
+ } else {
+ if (c.loopDepth >= 1)
+ NameOS << "_d" << c.loopDepth;
else
- addComponent('s', c.containerOpInRegionIdx);
+ NameOS << "_s" << c.idx;
}
}
- return Name.str().str();
+ return NameOS.str().str();
}
void OpenMPDialect::initialize() {
>From fdc54a666e5fd7dc363a4c4874faa712cfe28309 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 30 Sep 2025 11:42:33 +0200
Subject: [PATCH 09/11] flatten if-else chain
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 5d4039ba37b83..4dafb25d7d80f 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -268,14 +268,12 @@ static std::string generateLoopNestingName(StringRef prefix,
if (c.skip)
continue;
- if (c.isRegionArgOfOp) {
+ if (c.isRegionArgOfOp)
NameOS << "_r" << c.idx;
- } else {
- if (c.loopDepth >= 1)
- NameOS << "_d" << c.loopDepth;
- else
- NameOS << "_s" << c.idx;
- }
+ else if (c.loopDepth >= 1)
+ NameOS << "_d" << c.loopDepth;
+ else
+ NameOS << "_s" << c.idx;
}
return NameOS.str().str();
>From 757117dea193a18cc9a2e3f4d86dfe25b9727746 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Tue, 30 Sep 2025 12:15:28 +0200
Subject: [PATCH 10/11] Move fir test to mlir openmp using scf.if
---
flang/test/Fir/omp-cli.fir | 28 ------------------
.../Dialect/OpenMP/cli-canonical_loop.mlir | 29 ++++++++++++++++++-
2 files changed, 28 insertions(+), 29 deletions(-)
delete mode 100644 flang/test/Fir/omp-cli.fir
diff --git a/flang/test/Fir/omp-cli.fir b/flang/test/Fir/omp-cli.fir
deleted file mode 100644
index 8e5ea00b9744a..0000000000000
--- a/flang/test/Fir/omp-cli.fir
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: fir-opt %s | FileCheck %s --enable-var-scope
-// RUN: fir-opt %s | fir-opt | FileCheck %s --enable-var-scope
-
-// CHECK-LABEL: @omp_canonloop_multiregion(
-func.func @omp_canonloop_multiregion(%c : i1) -> () {
- %c42_i32 = arith.constant 42: i32
- %canonloop1 = omp.new_cli
- %canonloop2 = omp.new_cli
- %canonloop3 = omp.new_cli
- fir.if %c {
- // CHECK: omp.canonical_loop(%canonloop_r0) %iv_r0 : i32 in range(%c42_i32) {
- omp.canonical_loop(%canonloop1) %iv1 : i32 in range(%c42_i32) {
- omp.terminator
- }
- } else {
- // CHECK: omp.canonical_loop(%canonloop_r1_s0) %iv_r1_s0 : i32 in range(%c42_i32) {
- omp.canonical_loop(%canonloop2) %iv2 : i32 in range(%c42_i32) {
- omp.terminator
- }
- // CHECK: omp.canonical_loop(%canonloop_r1_s1) %iv_r1_s1 : i32 in range(%c42_i32) {
- omp.canonical_loop(%canonloop3) %iv3 : i32 in range(%c42_i32) {
- omp.terminator
- }
- }
-
- // CHECK: fir.unreachable
- fir.unreachable
-}
diff --git a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
index c77253cea94fe..0e9385ee75c47 100644
--- a/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
+++ b/mlir/test/Dialect/OpenMP/cli-canonical_loop.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s | FileCheck %s --enable-var-scope
+// RUN: mlir-opt %s | FileCheck %s --enable-var-scope
// RUN: mlir-opt %s | mlir-opt | FileCheck %s --enable-var-scope
@@ -278,3 +278,30 @@ func.func @omp_canonloop_multiregion_isolatedfromabove() -> () {
// CHECK: return
return
}
+
+
+// CHECK-LABEL: @omp_canonloop_multiregion(
+func.func @omp_canonloop_multiregion(%c : i1) -> () {
+ %c42_i32 = arith.constant 42: i32
+ %canonloop1 = omp.new_cli
+ %canonloop2 = omp.new_cli
+ %canonloop3 = omp.new_cli
+ scf.if %c {
+ // CHECK: omp.canonical_loop(%canonloop_r0) %iv_r0 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop1) %iv1 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ } else {
+ // CHECK: omp.canonical_loop(%canonloop_r1_s0) %iv_r1_s0 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop2) %iv2 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ // CHECK: omp.canonical_loop(%canonloop_r1_s1) %iv_r1_s1 : i32 in range(%c42_i32) {
+ omp.canonical_loop(%canonloop3) %iv3 : i32 in range(%c42_i32) {
+ omp.terminator
+ }
+ }
+
+ // CHECK: return
+ return
+}
>From b2847e763f3afd032320097ee79bced222a555d4 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Thu, 2 Oct 2025 14:54:03 +0200
Subject: [PATCH 11/11] Avoid use of anon struct/union
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 80 +++++++++++++-------
1 file changed, 52 insertions(+), 28 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 81c292a038469..799e3b9b1f4e1 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -94,23 +94,47 @@ struct LLVMPointerPointerLikeModel
static std::string generateLoopNestingName(StringRef prefix,
CanonicalLoopOp op) {
struct Component {
+ /// If true, this component describes a region operand of an operation (the
+ /// operand's owner) If false, this component describes an operation located
+ /// in a parent region
bool isRegionArgOfOp;
bool skip = false;
bool isUnique = false;
+
size_t idx;
+ Operation *op;
+ Region *parentRegion;
+ size_t loopDepth;
- union {
- /// isRegionArgOfOp == true: region argument of an operation
- struct {
- Operation *ownerOp;
- };
- /// isRegionArgOfOp == false: container op in a region
- struct {
- Operation *containerOp;
- Region *parentRegion;
- size_t loopDepth;
- };
- };
+ Operation *&getOwnerOp() {
+ assert(isRegionArgOfOp && "Must describe a region operand");
+ return op;
+ }
+ size_t &getArgIdx() {
+ assert(isRegionArgOfOp && "Must describe a region operand");
+ return idx;
+ }
+
+ Operation *&getContainerOp() {
+ assert(!isRegionArgOfOp && "Must describe a operation of a region");
+ return op;
+ }
+ size_t &getOpPos() {
+ assert(!isRegionArgOfOp && "Must describe a operation of a region");
+ return idx;
+ }
+ bool isLoopOp() const {
+ assert(!isRegionArgOfOp && "Must describe a operation of a region");
+ return isa<CanonicalLoopOp>(op);
+ }
+ Region *&getParentRegion() {
+ assert(!isRegionArgOfOp && "Must describe a operation of a region");
+ return parentRegion;
+ }
+ size_t &getLoopDepth() {
+ assert(!isRegionArgOfOp && "Must describe a operation of a region");
+ return loopDepth;
+ }
void skipIf(bool v = true) { skip = skip || v; }
};
@@ -153,19 +177,19 @@ static std::string generateLoopNestingName(StringRef prefix,
Component &containerOpInRegion = components.emplace_back();
containerOpInRegion.isRegionArgOfOp = false;
- containerOpInRegion.containerOp = o;
- containerOpInRegion.parentRegion = r;
- containerOpInRegion.idx = sequentialIdx;
containerOpInRegion.isUnique = isOnlyContainerOp;
+ containerOpInRegion.getContainerOp() = o;
+ containerOpInRegion.getOpPos() = sequentialIdx;
+ containerOpInRegion.getParentRegion() = r;
Operation *parent = r->getParentOp();
// Region argument of an operation
Component ®ionArgOfOperation = components.emplace_back();
regionArgOfOperation.isRegionArgOfOp = true;
- regionArgOfOperation.ownerOp = parent;
- regionArgOfOperation.idx = 0;
regionArgOfOperation.isUnique = true;
+ regionArgOfOperation.getArgIdx() = 0;
+ regionArgOfOperation.getOwnerOp() = parent;
// The IsolatedFromAbove trait of the parent operation implies that each
// individual region argument has its own separate namespace, so no
@@ -183,8 +207,8 @@ static std::string generateLoopNestingName(StringRef prefix,
}
llvm_unreachable("Region not child of its parent operation");
};
- regionArgOfOperation.idx = getRegionIndex(parent, r);
regionArgOfOperation.isUnique = false;
+ regionArgOfOperation.getArgIdx() = getRegionIndex(parent, r);
}
// next parent
@@ -212,10 +236,10 @@ static std::string generateLoopNestingName(StringRef prefix,
if (!c.isUnique)
numSurroundingLoops = 0;
- c.loopDepth = numSurroundingLoops;
+ c.getLoopDepth() = numSurroundingLoops;
// Next loop is surrounded by one more loop
- if (isa<CanonicalLoopOp>(c.containerOp))
+ if (isa<CanonicalLoopOp>(c.getContainerOp()))
numSurroundingLoops += 1;
}
@@ -226,7 +250,7 @@ static std::string generateLoopNestingName(StringRef prefix,
if (c.skip || c.isRegionArgOfOp)
continue;
- if (!isLoopNest && c.loopDepth >= 1) {
+ if (!isLoopNest && c.getLoopDepth() >= 1) {
// Innermost loop of a loop nest of at least two loops
isLoopNest = true;
} else if (isLoopNest) {
@@ -235,7 +259,7 @@ static std::string generateLoopNestingName(StringRef prefix,
// If there is no surrounding loop left, this must have been the outermost
// loop; leave loop-nest mode for the next iteration
- if (c.loopDepth == 0)
+ if (c.getLoopDepth() == 0)
isLoopNest = false;
}
}
@@ -244,7 +268,7 @@ static std::string generateLoopNestingName(StringRef prefix,
// we mark them as skipped only after computing loop nests)
for (Component &c : components)
c.skipIf(!c.isRegionArgOfOp && c.isUnique &&
- !isa<CanonicalLoopOp>(c.containerOp));
+ !isa<CanonicalLoopOp>(c.getContainerOp()));
// Components can be skipped if they are already disambiguated by their parent
// (or does not have a parent)
@@ -257,7 +281,7 @@ static std::string generateLoopNestingName(StringRef prefix,
newRegion = true;
// ...except canonical loops that need a suffix for each nest
- if (!c.isRegionArgOfOp && isa<CanonicalLoopOp>(c.containerOp))
+ if (!c.isRegionArgOfOp && c.getContainerOp())
newRegion = false;
}
@@ -269,11 +293,11 @@ static std::string generateLoopNestingName(StringRef prefix,
continue;
if (c.isRegionArgOfOp)
- NameOS << "_r" << c.idx;
- else if (c.loopDepth >= 1)
- NameOS << "_d" << c.loopDepth;
+ NameOS << "_r" << c.getArgIdx();
+ else if (c.getLoopDepth() >= 1)
+ NameOS << "_d" << c.getLoopDepth();
else
- NameOS << "_s" << c.idx;
+ NameOS << "_s" << c.getOpPos();
}
return NameOS.str().str();
More information about the Mlir-commits
mailing list