From b3919715ebe223b39dd789dcd471a864666d7008 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project@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 (&region == 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 (&region == 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 &region,
                                                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@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@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@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@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 (&region == 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@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@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@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 &regionArgOfOperation = 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@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@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@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 &regionArgOfOperation = 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();