Skip to content

Commit 8dcee58

Browse files
[flang]Check for dominance in loop versioning (#68797)
This avoids trying to version loops that can't be versioned, and thus avoids hitting an assert. Co-authored with Slava Zakharin (who provided the test-code).
1 parent f35e2d8 commit 8dcee58

File tree

2 files changed

+54
-4
lines changed

2 files changed

+54
-4
lines changed

flang/lib/Optimizer/Transforms/LoopVersioning.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
5252
#include "flang/Optimizer/Transforms/Passes.h"
5353
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
54+
#include "mlir/IR/Dominance.h"
5455
#include "mlir/IR/Matchers.h"
5556
#include "mlir/IR/TypeUtilities.h"
5657
#include "mlir/Pass/Pass.h"
@@ -292,6 +293,8 @@ void LoopVersioningPass::runOnOperation() {
292293
// immediately nested in a loop.
293294
llvm::DenseMap<fir::DoLoopOp, ArgsUsageInLoop> argsInLoops;
294295

296+
auto &domInfo = getAnalysis<mlir::DominanceInfo>();
297+
295298
// Traverse the loops in post-order and see
296299
// if those arguments are used inside any loop.
297300
func.walk([&](fir::DoLoopOp loop) {
@@ -309,12 +312,15 @@ void LoopVersioningPass::runOnOperation() {
309312
for (auto a : argsOfInterest) {
310313
if (a.arg == normaliseVal(operand)) {
311314
// Use the reboxed value, not the block arg when re-creating the loop.
312-
// TODO: should we check that the operand dominates the loop?
313-
// If this might be a case, we should record such operands in
314-
// argsInLoop.cannotTransform, so that they disable the transformation
315-
// for the parent loops as well.
316315
a.arg = operand;
317316

317+
// Check that the operand dominates the loop?
318+
// If this is the case, record such operands in argsInLoop.cannot-
319+
// Transform, so that they disable the transformation for the parent
320+
/// loops as well.
321+
if (!domInfo.dominates(a.arg, loop))
322+
argsInLoop.cannotTransform.insert(a.arg);
323+
318324
// No support currently for sliced arrays.
319325
// This means that we cannot transform properly
320326
// instructions referencing a.arg in the whole loop

flang/test/Transforms/loop-versioning.fir

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,4 +1434,48 @@ func.func @_QPtest_loop_nest(%arg0: !fir.box<!fir.array<?xf32>> {fir.bindc_name
14341434
// CHECK: fir.if
14351435
// CHECK-NOT: fir.if
14361436

1437+
1438+
//-----
1439+
1440+
// Check that a non-dominating operand isn't causing a problem.
1441+
// Just check it compiles, and doesn't version this loop.
1442+
func.func @sum1drebox(%arg0: !fir.box<!fir.array<?xf64>> {fir.bindc_name = "a"}, %arg1: !fir.ref<i32> {fir.bindc_name = "n"}) {
1443+
%decl = fir.declare %arg0 {uniq_name = "a"} : (!fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>>
1444+
%0 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QMmoduleFsum1dEi"}
1445+
%1 = fir.alloca f64 {bindc_name = "sum", uniq_name = "_QMmoduleFsum1dEsum"}
1446+
%cst = arith.constant 0.000000e+00 : f64
1447+
fir.store %cst to %1 : !fir.ref<f64>
1448+
%c1_i32 = arith.constant 1 : i32
1449+
%2 = fir.convert %c1_i32 : (i32) -> index
1450+
%3 = fir.load %arg1 : !fir.ref<i32>
1451+
%4 = fir.convert %3 : (i32) -> index
1452+
%c1 = arith.constant 1 : index
1453+
%5 = fir.convert %2 : (index) -> i32
1454+
%6:2 = fir.do_loop %arg2 = %2 to %4 step %c1 iter_args(%arg3 = %5) -> (index, i32) {
1455+
// rebox is not dominating the loop.
1456+
%rebox = fir.rebox %decl : (!fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>>
1457+
fir.store %arg3 to %0 : !fir.ref<i32>
1458+
%7 = fir.load %1 : !fir.ref<f64>
1459+
%8 = fir.load %0 : !fir.ref<i32>
1460+
%9 = fir.convert %8 : (i32) -> i64
1461+
%c1_i64 = arith.constant 1 : i64
1462+
%10 = arith.subi %9, %c1_i64 : i64
1463+
%11 = fir.coordinate_of %rebox, %10 : (!fir.box<!fir.array<?xf64>>, i64) -> !fir.ref<f64>
1464+
%12 = fir.load %11 : !fir.ref<f64>
1465+
%13 = arith.addf %7, %12 fastmath<contract> : f64
1466+
fir.store %13 to %1 : !fir.ref<f64>
1467+
%14 = arith.addi %arg2, %c1 : index
1468+
%15 = fir.convert %c1 : (index) -> i32
1469+
%16 = fir.load %0 : !fir.ref<i32>
1470+
%17 = arith.addi %16, %15 : i32
1471+
fir.result %14, %17 : index, i32
1472+
}
1473+
fir.store %6#1 to %0 : !fir.ref<i32>
1474+
return
1475+
}
1476+
// CHECK-LABEL: func @sum1drebox
1477+
// No versioning -> no if-operation.
1478+
// CHECK-NOT: fir.if
1479+
1480+
14371481
} // End module

0 commit comments

Comments
 (0)