[Mlir-commits] [mlir] 3628feb - [mlir] NFC control-flow sink cleanup

llvmlistbot at llvm.org llvmlistbot at llvm.org
Mon Jan 24 15:34:55 PST 2022


Author: Mogball
Date: 2022-01-24T23:34:42Z
New Revision: 3628febcf8e3d88db5871c9f82a33ab98611e5a8

URL: https://github.com/llvm/llvm-project/commit/3628febcf8e3d88db5871c9f82a33ab98611e5a8
DIFF: https://github.com/llvm/llvm-project/commit/3628febcf8e3d88db5871c9f82a33ab98611e5a8.diff

LOG: [mlir] NFC control-flow sink cleanup

Added: 
    

Modified: 
    mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
    mlir/include/mlir/Transforms/Passes.td
    mlir/lib/Transforms/ControlFlowSink.cpp
    mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp
    mlir/test/Transforms/control-flow-sink.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
index 429a5356428f7..896eb501d3799 100644
--- a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
+++ b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
@@ -148,7 +148,8 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
       (ins "::mlir::ArrayRef<::mlir::Attribute>":$operands,
            "::llvm::SmallVectorImpl<::mlir::InvocationBounds> &"
              :$invocationBounds), [{}],
-       [{ invocationBounds.append($_op->getNumRegions(), {0, ::llvm::None}); }]
+       [{ invocationBounds.append($_op->getNumRegions(),
+                                  ::mlir::InvocationBounds::getUnknown()); }]
     >,
   ];
 

diff  --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td
index 56e90644363e5..4b1d6ca71e2e1 100644
--- a/mlir/include/mlir/Transforms/Passes.td
+++ b/mlir/include/mlir/Transforms/Passes.td
@@ -310,14 +310,16 @@ def Canonicalizer : Pass<"canonicalize"> {
 def ControlFlowSink : Pass<"control-flow-sink"> {
   let summary = "Sink operations into conditional blocks";
   let description = [{
-    This pass implements a simple control-flow sink on operations that implement
+    This pass implements control-flow sink on operations that implement
     `RegionBranchOpInterface` by moving dominating operations whose only uses
-    are in a single conditionally-executed region into that region so that
+    are in a conditionally-executed regions into those regions so that
     executions paths where their results are not needed do not perform
     unnecessary computations.
 
     This is similar (but opposite) to loop-invariant code motion, which hoists
-    operations out of regions executed more than once.
+    operations out of regions executed more than once. The implementation of
+    control-flow sink uses a simple and conversative cost model: operations are
+    never duplicated and are only moved into singly-executed regions.
 
     It is recommended to run canonicalization first to remove unreachable
     blocks: ops in unreachable blocks may prevent other operations from being

diff  --git a/mlir/lib/Transforms/ControlFlowSink.cpp b/mlir/lib/Transforms/ControlFlowSink.cpp
index 71afc5702edf0..10d41b0f0013f 100644
--- a/mlir/lib/Transforms/ControlFlowSink.cpp
+++ b/mlir/lib/Transforms/ControlFlowSink.cpp
@@ -22,9 +22,7 @@
 using namespace mlir;
 
 namespace {
-/// A basic control-flow sink pass. This pass analyzes the regions of operations
-/// that implement `RegionBranchOpInterface` that are reachable and executed at
-/// most once and sinks candidate operations that are side-effect free.
+/// A control-flow sink pass.
 struct ControlFlowSink : public ControlFlowSinkBase<ControlFlowSink> {
   void runOnOperation() override;
 };
@@ -59,10 +57,13 @@ void ControlFlowSink::runOnOperation() {
   auto &domInfo = getAnalysis<DominanceInfo>();
   getOperation()->walk([&](RegionBranchOpInterface branch) {
     SmallVector<Region *> regionsToSink;
+    // Get the regions are that known to be executed at most once.
     getSinglyExecutedRegionsToSink(branch, regionsToSink);
-    numSunk = mlir::controlFlowSink(
-        regionsToSink, domInfo,
-        [](Operation *op, Region *) { return isSideEffectFree(op); });
+    // Sink side-effect free operations.
+    numSunk =
+        controlFlowSink(regionsToSink, domInfo, [](Operation *op, Region *) {
+          return isSideEffectFree(op);
+        });
   });
 }
 

diff  --git a/mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp b/mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp
index cffbd922f88c8..868174b261c4a 100644
--- a/mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp
+++ b/mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file implements utilityies for control-flow sinking. Control-flow
+// This file implements utilities for control-flow sinking. Control-flow
 // sinking moves operations whose only uses are in conditionally-executed blocks
 // into those blocks so that they aren't executed on paths where their results
 // are not needed.
@@ -40,7 +40,7 @@ class Sinker {
 
   /// Given a list of regions, find operations to sink and sink them. Return the
   /// number of operations sunk.
-  size_t sinkRegions(ArrayRef<Region *> regions) &&;
+  size_t sinkRegions(ArrayRef<Region *> regions);
 
 private:
   /// Given a region and an op which dominates the region, returns true if all
@@ -93,9 +93,9 @@ void Sinker::tryToSinkPredecessors(Operation *user, Region *region,
     if (allUsersDominatedBy(op, region) && shouldMoveIntoRegion(op, region)) {
       // Move the op into the region's entry block. If the op is part of a
       // subgraph, dependee ops would have been moved first, so inserting before
-      // the start of the block will ensure dominance is preserved. Ops can only
-      // be safely moved into the entry block as the region's other blocks may
-      // for a loop.
+      // the start of the block will ensure SSA dominance is preserved locally
+      // in the subgraph. Ops can only be safely moved into the entry block as
+      // the region's other blocks may for a loop.
       op->moveBefore(&region->front(), region->front().begin());
       ++numSunk;
       // Add the op to the work queue.
@@ -119,7 +119,7 @@ void Sinker::sinkRegion(Region *region) {
   }
 }
 
-size_t Sinker::sinkRegions(ArrayRef<Region *> regions) && {
+size_t Sinker::sinkRegions(ArrayRef<Region *> regions) {
   for (Region *region : regions)
     if (!region->empty())
       sinkRegion(region);
@@ -137,7 +137,8 @@ void mlir::getSinglyExecutedRegionsToSink(RegionBranchOpInterface branch,
   // Collect constant operands.
   SmallVector<Attribute> operands(branch->getNumOperands(), Attribute());
   for (auto &it : llvm::enumerate(branch->getOperands()))
-    matchPattern(it.value(), m_Constant(&operands[it.index()]));
+    (void)matchPattern(it.value(), m_Constant(&operands[it.index()]));
+
   // Get the invocation bounds.
   SmallVector<InvocationBounds> bounds;
   branch.getRegionInvocationBounds(operands, bounds);

diff  --git a/mlir/test/Transforms/control-flow-sink.mlir b/mlir/test/Transforms/control-flow-sink.mlir
index 58dffe19c0aae..ba895ebc493e5 100644
--- a/mlir/test/Transforms/control-flow-sink.mlir
+++ b/mlir/test/Transforms/control-flow-sink.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -split-input-file -control-flow-sink %s | FileCheck %s
+// RUN: mlir-opt -control-flow-sink %s | FileCheck %s
 
 // Test that operations can be sunk.
 
@@ -35,8 +35,6 @@ func @test_simple_sink(%arg0: i1, %arg1: i32, %arg2: i32) -> i32 {
   return %4 : i32
 }
 
-// -----
-
 // Test that a region op can be sunk.
 
 // CHECK-LABEL: @test_region_sink
@@ -76,8 +74,6 @@ func @test_region_sink(%arg0: i1, %arg1: i32, %arg2: i32) -> i32 {
   return %2 : i32
 }
 
-// -----
-
 // Test that an entire subgraph can be sunk.
 
 // CHECK-LABEL: @test_subgraph_sink
@@ -113,8 +109,6 @@ func @test_subgraph_sink(%arg0: i1, %arg1: i32, %arg2: i32) -> i32 {
   return %6 : i32
 }
 
-// -----
-
 // Test that ops can be sunk into regions with multiple blocks.
 
 // CHECK-LABEL: @test_multiblock_region_sink
@@ -144,8 +138,6 @@ func @test_multiblock_region_sink(%arg0: i1, %arg1: i32, %arg2: i32) -> i32 {
   return %4 : i32
 }
 
-// -----
-
 // Test that ops can be sunk recursively into nested regions.
 
 // CHECK-LABEL: @test_nested_region_sink
@@ -185,8 +177,6 @@ func @test_nested_region_sink(%arg0: i1, %arg1: i32) -> i32 {
   return %1 : i32
 }
 
-// -----
-
 // Test that ops are only moved into the entry block, even when their only uses
 // are further along.
 


        


More information about the Mlir-commits mailing list