Skip to content

Commit 3430ae1

Browse files
committed
[mlir] Update LICM to support Graph Regions
Changes the algorithm of LICM to support graph regions (no guarantee of topologically sorted order). Also fixes an issue where ops with recursive side effects and regions would not be hoisted if any nested ops used operands that were defined within the nested region. Reviewed By: rriddle Differential Revision: https://reviews.llvm.org/D122465
1 parent 04e094a commit 3430ae1

File tree

3 files changed

+210
-69
lines changed

3 files changed

+210
-69
lines changed

mlir/lib/Interfaces/LoopLikeInterface.cpp

Lines changed: 68 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "mlir/Interfaces/SideEffectInterfaces.h"
1111
#include "llvm/ADT/SmallPtrSet.h"
1212
#include "llvm/Support/Debug.h"
13+
#include <queue>
1314

1415
using namespace mlir;
1516

@@ -26,75 +27,88 @@ using namespace mlir;
2627
// LoopLike Utilities
2728
//===----------------------------------------------------------------------===//
2829

29-
// Checks whether the given op can be hoisted by checking that
30-
// - the op and any of its contained operations do not depend on SSA values
31-
// defined inside of the loop (by means of calling definedOutside).
32-
// - the op has no side-effects. If sideEffecting is Never, sideeffects of this
33-
// op and its nested ops are ignored.
34-
static bool canBeHoisted(Operation *op,
35-
function_ref<bool(Value)> definedOutside) {
36-
// Check that dependencies are defined outside of loop.
37-
if (!llvm::all_of(op->getOperands(), definedOutside))
38-
return false;
39-
// Check whether this op is side-effect free. If we already know that there
40-
// can be no side-effects because the surrounding op has claimed so, we can
41-
// (and have to) skip this step.
30+
/// Returns true if the given operation is side-effect free as are all of its
31+
/// nested operations.
32+
///
33+
/// TODO: There is a duplicate function in ControlFlowSink. Move
34+
/// `moveLoopInvariantCode` to TransformUtils and then factor out this function.
35+
static bool isSideEffectFree(Operation *op) {
4236
if (auto memInterface = dyn_cast<MemoryEffectOpInterface>(op)) {
37+
// If the op has side-effects, it cannot be moved.
4338
if (!memInterface.hasNoEffect())
4439
return false;
45-
// If the operation doesn't have side effects and it doesn't recursively
46-
// have side effects, it can always be hoisted.
40+
// If the op does not have recursive side effects, then it can be moved.
4741
if (!op->hasTrait<OpTrait::HasRecursiveSideEffects>())
4842
return true;
49-
50-
// Otherwise, if the operation doesn't provide the memory effect interface
51-
// and it doesn't have recursive side effects we treat it conservatively as
52-
// side-effecting.
5343
} else if (!op->hasTrait<OpTrait::HasRecursiveSideEffects>()) {
44+
// Otherwise, if the op does not implement the memory effect interface and
45+
// it does not have recursive side effects, then it cannot be known that the
46+
// op is moveable.
5447
return false;
5548
}
5649

57-
// Recurse into the regions for this op and check whether the contained ops
58-
// can be hoisted.
59-
for (auto &region : op->getRegions()) {
60-
for (auto &block : region) {
61-
for (auto &innerOp : block)
62-
if (!canBeHoisted(&innerOp, definedOutside))
63-
return false;
64-
}
65-
}
50+
// Recurse into the regions and ensure that all nested ops can also be moved.
51+
for (Region &region : op->getRegions())
52+
for (Operation &op : region.getOps())
53+
if (!isSideEffectFree(&op))
54+
return false;
6655
return true;
6756
}
6857

58+
/// Checks whether the given op can be hoisted by checking that
59+
/// - the op and none of its contained operations depend on values inside of the
60+
/// loop (by means of calling definedOutside).
61+
/// - the op has no side-effects.
62+
static bool canBeHoisted(Operation *op,
63+
function_ref<bool(Value)> definedOutside) {
64+
if (!isSideEffectFree(op))
65+
return false;
66+
67+
// Do not move terminators.
68+
if (op->hasTrait<OpTrait::IsTerminator>())
69+
return false;
70+
71+
// Walk the nested operations and check that all used values are either
72+
// defined outside of the loop or in a nested region, but not at the level of
73+
// the loop body.
74+
auto walkFn = [&](Operation *child) {
75+
for (Value operand : child->getOperands()) {
76+
// Ignore values defined in a nested region.
77+
if (op->isAncestor(operand.getParentRegion()->getParentOp()))
78+
continue;
79+
if (!definedOutside(operand))
80+
return WalkResult::interrupt();
81+
}
82+
return WalkResult::advance();
83+
};
84+
return !op->walk(walkFn).wasInterrupted();
85+
}
86+
6987
void mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
70-
auto &loopBody = looplike.getLoopBody();
71-
72-
// We use two collections here as we need to preserve the order for insertion
73-
// and this is easiest.
74-
SmallPtrSet<Operation *, 8> willBeMovedSet;
75-
SmallVector<Operation *, 8> opsToMove;
76-
77-
// Helper to check whether an operation is loop invariant wrt. SSA properties.
78-
auto isDefinedOutsideOfBody = [&](Value value) {
79-
auto *definingOp = value.getDefiningOp();
80-
return (definingOp && !!willBeMovedSet.count(definingOp)) ||
81-
looplike.isDefinedOutsideOfLoop(value);
88+
Region *loopBody = &looplike.getLoopBody();
89+
90+
std::queue<Operation *> worklist;
91+
// Add top-level operations in the loop body to the worklist.
92+
for (Operation &op : loopBody->getOps())
93+
worklist.push(&op);
94+
95+
auto definedOutside = [&](Value value) {
96+
return looplike.isDefinedOutsideOfLoop(value);
8297
};
8398

84-
// Do not use walk here, as we do not want to go into nested regions and hoist
85-
// operations from there. These regions might have semantics unknown to this
86-
// rewriting. If the nested regions are loops, they will have been processed.
87-
for (auto &block : loopBody) {
88-
for (auto &op : block.without_terminator()) {
89-
if (canBeHoisted(&op, isDefinedOutsideOfBody)) {
90-
opsToMove.push_back(&op);
91-
willBeMovedSet.insert(&op);
92-
}
93-
}
94-
}
99+
while (!worklist.empty()) {
100+
Operation *op = worklist.front();
101+
worklist.pop();
102+
// Skip ops that have already been moved. Check if the op can be hoisted.
103+
if (op->getParentRegion() != loopBody || !canBeHoisted(op, definedOutside))
104+
continue;
95105

96-
// For all instructions that we found to be invariant, move outside of the
97-
// loop.
98-
for (Operation *op : opsToMove)
99106
looplike.moveOutOfLoop(op);
107+
108+
// Since the op has been moved, we need to check its users within the
109+
// top-level of the loop body.
110+
for (Operation *user : op->getUsers())
111+
if (user->getParentRegion() == loopBody)
112+
worklist.push(user);
113+
}
100114
}

mlir/test/Transforms/loop-invariant-code-motion.mlir

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,25 +157,25 @@ func @invariant_affine_nested_if() {
157157
affine.for %arg0 = 0 to 10 {
158158
affine.for %arg1 = 0 to 10 {
159159
affine.if affine_set<(d0, d1) : (d1 - d0 >= 0)> (%arg0, %arg0) {
160-
%cf9 = arith.addf %cf8, %cf8 : f32
161-
affine.if affine_set<(d0, d1) : (d1 - d0 >= 0)> (%arg0, %arg0) {
162-
%cf10 = arith.addf %cf9, %cf9 : f32
163-
}
160+
%cf9 = arith.addf %cf8, %cf8 : f32
161+
affine.if affine_set<(d0, d1) : (d1 - d0 >= 0)> (%arg0, %arg0) {
162+
%cf10 = arith.addf %cf9, %cf9 : f32
163+
}
164164
}
165165
}
166166
}
167167

168168
// CHECK: memref.alloc
169169
// CHECK-NEXT: arith.constant
170170
// CHECK-NEXT: affine.for
171+
// CHECK-NEXT: }
171172
// CHECK-NEXT: affine.for
172173
// CHECK-NEXT: affine.if
173174
// CHECK-NEXT: arith.addf
174175
// CHECK-NEXT: affine.if
175176
// CHECK-NEXT: arith.addf
176177
// CHECK-NEXT: }
177178
// CHECK-NEXT: }
178-
// CHECK-NEXT: }
179179

180180

181181
return
@@ -319,6 +319,110 @@ func @nested_uses_inside(%lb: index, %ub: index, %step: index) {
319319
scf.yield %val2: index
320320
}
321321
}
322-
return
322+
return
323323
}
324324

325+
// -----
326+
327+
// Test that two ops that feed into each other are moved without violating
328+
// dominance in non-graph regions.
329+
// CHECK-LABEL: func @invariant_subgraph
330+
// CHECK-SAME: %{{.*}}: index, %{{.*}}: index, %{{.*}}: index, %[[ARG:.*]]: i32
331+
func @invariant_subgraph(%lb: index, %ub: index, %step: index, %arg: i32) {
332+
// CHECK: %[[V0:.*]] = arith.addi %[[ARG]], %[[ARG]]
333+
// CHECK-NEXT: %[[V1:.*]] = arith.addi %[[ARG]], %[[V0]]
334+
// CHECK-NEXT: scf.for
335+
scf.for %i = %lb to %ub step %step {
336+
// CHECK-NEXT: "test.sink"(%[[V1]])
337+
%v0 = arith.addi %arg, %arg : i32
338+
%v1 = arith.addi %arg, %v0 : i32
339+
"test.sink"(%v1) : (i32) -> ()
340+
}
341+
return
342+
}
343+
344+
// -----
345+
346+
// Test invariant nested loop is hoisted.
347+
// CHECK-LABEL: func @test_invariant_nested_loop
348+
func @test_invariant_nested_loop() {
349+
// CHECK: %[[C:.*]] = arith.constant
350+
%0 = arith.constant 5 : i32
351+
// CHECK: %[[V0:.*]] = arith.addi %[[C]], %[[C]]
352+
// CHECK-NEXT: %[[V1:.*]] = arith.addi %[[V0]], %[[C]]
353+
// CHECK-NEXT: test.graph_loop
354+
// CHECK-NEXT: ^bb0(%[[ARG0:.*]]: i32)
355+
// CHECK-NEXT: %[[V2:.*]] = arith.subi %[[ARG0]], %[[ARG0]]
356+
// CHECK-NEXT: test.region_yield %[[V2]]
357+
// CHECK: test.graph_loop
358+
// CHECK-NEXT: test.region_yield %[[V1]]
359+
test.graph_loop {
360+
%1 = arith.addi %0, %0 : i32
361+
%2 = arith.addi %1, %0 : i32
362+
test.graph_loop {
363+
^bb0(%arg0: i32):
364+
%3 = arith.subi %arg0, %arg0 : i32
365+
test.region_yield %3 : i32
366+
} : () -> ()
367+
test.region_yield %2 : i32
368+
} : () -> ()
369+
return
370+
}
371+
372+
373+
// -----
374+
375+
// Test ops in a graph region are hoisted.
376+
// CHECK-LABEL: func @test_invariants_in_graph_region
377+
func @test_invariants_in_graph_region() {
378+
// CHECK: test.single_no_terminator_op
379+
test.single_no_terminator_op : {
380+
// CHECK-NEXT: %[[C:.*]] = arith.constant
381+
// CHECK-NEXT: %[[V1:.*]] = arith.addi %[[C]], %[[C]]
382+
// CHECK-NEXT: %[[V0:.*]] = arith.addi %[[C]], %[[V1]]
383+
test.graph_loop {
384+
%v0 = arith.addi %c0, %v1 : i32
385+
%v1 = arith.addi %c0, %c0 : i32
386+
%c0 = arith.constant 5 : i32
387+
test.region_yield %v0 : i32
388+
} : () -> ()
389+
}
390+
return
391+
}
392+
393+
// -----
394+
395+
// Test ops in a graph region are hoisted in topological order into non-graph
396+
// regions and that dominance is preserved.
397+
// CHECK-LABEL: func @test_invariant_backedge
398+
func @test_invariant_backedge() {
399+
// CHECK-NEXT: %[[C:.*]] = arith.constant
400+
// CHECK-NEXT: %[[V1:.*]] = arith.addi %[[C]], %[[C]]
401+
// CHECK-NEXT: %[[V0:.*]] = arith.addi %[[C]], %[[V1]]
402+
// CHECK-NEXT: test.graph_loop
403+
test.graph_loop {
404+
// CHECK-NEXT: test.region_yield %[[V0]]
405+
%v0 = arith.addi %c0, %v1 : i32
406+
%v1 = arith.addi %c0, %c0 : i32
407+
%c0 = arith.constant 5 : i32
408+
test.region_yield %v0 : i32
409+
} : () -> ()
410+
return
411+
}
412+
413+
// -----
414+
415+
// Test that cycles aren't hoisted from graph regions to non-graph regions.
416+
// CHECK-LABEL: func @test_invariant_cycle_not_hoisted
417+
func @test_invariant_cycle_not_hoisted() {
418+
// CHECK: test.graph_loop
419+
test.graph_loop {
420+
// CHECK-NEXT: %[[A:.*]] = "test.a"(%[[B:.*]]) :
421+
// CHECK-NEXT: %[[B]] = "test.b"(%[[A]]) :
422+
// CHECK-NEXT: test.region_yield %[[A]]
423+
%a = "test.a"(%b) : (i32) -> i32
424+
%b = "test.b"(%a) : (i32) -> i32
425+
test.region_yield %a : i32
426+
} : () -> ()
427+
return
428+
}

mlir/test/lib/Dialect/Test/TestOps.td

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010
#define TEST_OPS
1111

1212
include "TestDialect.td"
13+
include "TestInterfaces.td"
1314
include "mlir/Dialect/DLTI/DLTIBase.td"
15+
include "mlir/Dialect/Linalg/IR/LinalgInterfaces.td"
1416
include "mlir/IR/EnumAttr.td"
1517
include "mlir/IR/OpBase.td"
1618
include "mlir/IR/OpAsmInterface.td"
@@ -22,9 +24,8 @@ include "mlir/Interfaces/ControlFlowInterfaces.td"
2224
include "mlir/Interfaces/CopyOpInterface.td"
2325
include "mlir/Interfaces/DataLayoutInterfaces.td"
2426
include "mlir/Interfaces/InferTypeOpInterface.td"
27+
include "mlir/Interfaces/LoopLikeInterface.td"
2528
include "mlir/Interfaces/SideEffectInterfaces.td"
26-
include "mlir/Dialect/Linalg/IR/LinalgInterfaces.td"
27-
include "TestInterfaces.td"
2829

2930

3031
// Include the attribute definitions.
@@ -2759,14 +2760,14 @@ def : Pat<(TestDefaultStrAttrNoValueOp $value),
27592760
def TestResource : Resource<"TestResource">;
27602761

27612762
def TestEffectsOpA : TEST_Op<"op_with_effects_a"> {
2762-
let arguments = (ins
2763-
Arg<Variadic<AnyMemRef>, "", [MemRead]>,
2764-
Arg<FlatSymbolRefAttr, "", [MemRead]>:$first,
2765-
Arg<SymbolRefAttr, "", [MemWrite]>:$second,
2766-
Arg<OptionalAttr<SymbolRefAttr>, "", [MemRead]>:$optional_symbol
2767-
);
2763+
let arguments = (ins
2764+
Arg<Variadic<AnyMemRef>, "", [MemRead]>,
2765+
Arg<FlatSymbolRefAttr, "", [MemRead]>:$first,
2766+
Arg<SymbolRefAttr, "", [MemWrite]>:$second,
2767+
Arg<OptionalAttr<SymbolRefAttr>, "", [MemRead]>:$optional_symbol
2768+
);
27682769

2769-
let results = (outs Res<AnyMemRef, "", [MemAlloc<TestResource>]>);
2770+
let results = (outs Res<AnyMemRef, "", [MemAlloc<TestResource>]>);
27702771
}
27712772

27722773
def TestEffectsOpB : TEST_Op<"op_with_effects_b",
@@ -2792,4 +2793,26 @@ def TestVerifiersOp : TEST_Op<"verifiers",
27922793
let hasRegionVerifier = 1;
27932794
}
27942795

2796+
//===----------------------------------------------------------------------===//
2797+
// Test Loop Op with a graph region
2798+
//===----------------------------------------------------------------------===//
2799+
2800+
// Test loop op with a graph region.
2801+
def TestGraphLoopOp : TEST_Op<"graph_loop",
2802+
[LoopLikeOpInterface, NoSideEffect,
2803+
RecursiveSideEffects, SingleBlock,
2804+
RegionKindInterface, HasOnlyGraphRegion]> {
2805+
let arguments = (ins Variadic<AnyType>:$args);
2806+
let results = (outs Variadic<AnyType>:$rets);
2807+
let regions = (region SizedRegion<1>:$body);
2808+
2809+
let assemblyFormat = [{
2810+
$args $body attr-dict `:` functional-type(operands, results)
2811+
}];
2812+
2813+
let extraClassDeclaration = [{
2814+
mlir::Region &getLoopBody() { return getBody(); }
2815+
}];
2816+
}
2817+
27952818
#endif // TEST_OPS

0 commit comments

Comments
 (0)