[Mlir-commits] [mlir] 06c0d1b - [flang][acc] Remove dangling operands in OpenACCUtilsTiling (#208261)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Jul 9 07:35:40 PDT 2026
Author: Susan Tan (ス-ザン タン)
Date: 2026-07-09T10:35:35-04:00
New Revision: 06c0d1b70402c1c55cdb1975b2a43148dd4fe646
URL: https://github.com/llvm/llvm-project/commit/06c0d1b70402c1c55cdb1975b2a43148dd4fe646
DIFF: https://github.com/llvm/llvm-project/commit/06c0d1b70402c1c55cdb1975b2a43148dd4fe646.diff
LOG: [flang][acc] Remove dangling operands in OpenACCUtilsTiling (#208261)
When TILE has more dimensions than the loop's collapse count,
uncollapseLoops() synthesizes an inner loop via `createInnerLoop()`,
which
strips gang attributes from it but forgot to clear the gang operand
values. This left the loop with a non-empty gang operand list but no
device-type attribute, causing LoopOp::getGangValue() to dereference a
null attribute and crash with an assertion failure when the loop also
had
`GANG(STATIC:N) (or NUM/DIM)`.
Fix: also clear the gang operands when stripping gang attributes, so the
loop ends up fully gang-free.
Added:
Modified:
mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
index dddaebdee5ebc..09e83af2bc686 100644
--- a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
@@ -97,10 +97,20 @@ 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();
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 cd13b3d8427f0..a1f80ad9defdf 100644
--- a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
@@ -74,6 +74,98 @@ 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
+}
+
+// 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