[Mlir-commits] [mlir] [flang][acc] Remove dangling operands in OpenACCUtilsTiling (PR #208261)
Susan Tan ス-ザン タン
llvmlistbot at llvm.org
Wed Jul 8 10:48:43 PDT 2026
https://github.com/SusanTan updated https://github.com/llvm/llvm-project/pull/208261
>From 7428e41773155ef0216861f8f1166e15a3cfa167 Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Wed, 8 Jul 2026 09:39:14 -0700
Subject: [PATCH 1/3] add fix
---
mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
index dddaebdee5ebc..6452d7e0ed8c8 100644
--- a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
@@ -97,6 +97,13 @@ createInnerLoop(mlir::acc::LoopOp inputLoop, mlir::RewriterBase &rewriter,
elementLoop.removeGangOperandsArgTypeAttr();
elementLoop.removeGangOperandsSegmentsAttr();
elementLoop.removeGangOperandsDeviceTypeAttr();
+ // Also drop the operand values themselves so that elementLoop does not
+ // end up with a non-empty gang operand list but no corresponding
+ // device-type/segment/arg-type attributes. Leaving stale operands behind
+ // makes elementLoop look like it still has gang operands to later
+ // queries (e.g. LoopOp::getGangValue), which then dereference the
+ // now-missing device-type attribute.
+ elementLoop.getGangOperandsMutable().clear();
}
if (inputLoop.hasVector() || inputLoop.getVectorValue()) {
elementLoop.removeWorkerAttr();
>From 02340ce1bce6f42f8c41d295909f3c48fe8a9caf Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Wed, 8 Jul 2026 10:05:40 -0700
Subject: [PATCH 2/3] add test
---
.../test/Dialect/OpenACC/acc-loop-tiling.mlir | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
index cd13b3d8427f0..3b032959b1cc6 100644
--- a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
@@ -74,6 +74,49 @@ func.func @nested_loop_tile(%arg0: memref<100x50xf32>) {
return
}
+// Regression test: a loop with GANG(STATIC: N) combined with a multi-dim
+// TILE clause used to crash with an assertion failure inside
+// LoopOp::getGangValue() because uncollapseLoops() (needed here since the
+// tile count exceeds the implicit collapse count of 1) left one of the
+// generated inner loops with a leftover gang operand but no corresponding
+// gang device-type attribute. Check that the pass completes and that gang
+// is only preserved on the outermost tile loop.
+
+// CHECK-LABEL: func.func @gang_static_with_multi_dim_tile
+// CHECK: acc.loop gang({static=%{{.*}} : i32}) control(%[[I:.*]] : index) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: gang
+// CHECK: acc.loop control(%[[J:.*]] : index) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: gang
+// CHECK: acc.loop control({{.*}} : index) = (%[[I]] : index) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: gang
+// CHECK: acc.loop control({{.*}} : index) = (%[[J]] : index) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: gang
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+func.func @gang_static_with_multi_dim_tile(%arg0: memref<100x50xf32>) {
+ %c0 = arith.constant 0 : index
+ %c100 = arith.constant 100 : index
+ %c50 = arith.constant 50 : index
+ %c1 = arith.constant 1 : index
+ %c4 = arith.constant 4 : index
+ %c8 = arith.constant 8 : index
+ %cs = arith.constant 1 : i32
+ acc.loop gang({static=%cs : i32}) tile({%c4 : index, %c8 : index}) control(%i : index, %j : index) = (%c0, %c0 : index, index) to (%c100, %c50 : index, index) step (%c1, %c1 : index, index) {
+ %sum = arith.addi %i, %j : index
+ %val = arith.index_castui %sum : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i, %j] : memref<100x50xf32>
+ acc.yield
+ } attributes {independent = [#acc.device_type<none>]}
+ return
+}
+
// Test unknown tile size (*) represented as -1
// Should use default tile size (32)
>From 7a79628faffedbc0f8e3a47bd1d9940a7cad4190 Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Wed, 8 Jul 2026 10:48:30 -0700
Subject: [PATCH 3/3] remove worker value operands as well
---
.../OpenACC/Utils/OpenACCUtilsTiling.cpp | 3 ++
.../test/Dialect/OpenACC/acc-loop-tiling.mlir | 49 +++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
index 6452d7e0ed8c8..09e83af2bc686 100644
--- a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
@@ -108,6 +108,9 @@ createInnerLoop(mlir::acc::LoopOp inputLoop, mlir::RewriterBase &rewriter,
if (inputLoop.hasVector() || inputLoop.getVectorValue()) {
elementLoop.removeWorkerAttr();
elementLoop.removeWorkerNumOperandsDeviceTypeAttr();
+ // As above for gang, also drop the worker operand values so elementLoop
+ // does not keep a dangling worker operand with no device-type attribute.
+ elementLoop.getWorkerNumOperandsMutable().clear();
}
rewriter.finalizeOpModification(elementLoop);
diff --git a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
index 3b032959b1cc6..a1f80ad9defdf 100644
--- a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
@@ -117,6 +117,55 @@ func.func @gang_static_with_multi_dim_tile(%arg0: memref<100x50xf32>) {
return
}
+// Regression test: a loop with WORKER(N) combined with VECTOR and a
+// multi-dim TILE clause used to produce invalid IR because createInnerLoop()
+// (via uncollapseLoops()) and removeWorkerVectorFromLoop() removed the
+// worker attributes from generated inner/tile loops without also clearing
+// the worker operand value copied onto them. This left loops with a
+// non-empty worker operand list but no worker device-type attribute,
+// which the verifier rejects ('worker operands count must match worker
+// device_type count'). Check that the pass produces valid IR, with worker
+// only preserved on the outermost tile loop and vector only on the
+// outermost element loop.
+
+// CHECK-LABEL: func.func @worker_num_with_multi_dim_tile
+// CHECK: acc.loop worker(%{{.*}} : i32) control(%[[I:.*]] : index) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: worker
+// CHECK-NOT: vector
+// CHECK: acc.loop control(%[[J:.*]] : index) = ({{.*}}) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: worker
+// CHECK-NOT: vector
+// CHECK: acc.loop vector control({{.*}} : index) = (%[[I]] : index) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: worker
+// CHECK: acc.loop control({{.*}} : index) = (%[[J]] : index) to ({{.*}}) step ({{.*}}) {
+// CHECK-NOT: worker
+// CHECK-NOT: vector
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+// CHECK: acc.yield
+// CHECK: }
+func.func @worker_num_with_multi_dim_tile(%arg0: memref<100x50xf32>) {
+ %c0 = arith.constant 0 : index
+ %c100 = arith.constant 100 : index
+ %c50 = arith.constant 50 : index
+ %c1 = arith.constant 1 : index
+ %c4 = arith.constant 4 : index
+ %c8 = arith.constant 8 : index
+ %cw = arith.constant 4 : i32
+ acc.loop worker(%cw : i32) vector tile({%c4 : index, %c8 : index}) control(%i : index, %j : index) = (%c0, %c0 : index, index) to (%c100, %c50 : index, index) step (%c1, %c1 : index, index) {
+ %sum = arith.addi %i, %j : index
+ %val = arith.index_castui %sum : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i, %j] : memref<100x50xf32>
+ acc.yield
+ } attributes {independent = [#acc.device_type<none>]}
+ return
+}
+
// Test unknown tile size (*) represented as -1
// Should use default tile size (32)
More information about the Mlir-commits
mailing list