[Mlir-commits] [mlir] 6139463 - [mlir] Ignore effects on allocated results when checking whether the op is trivially dead.

[Markus Böck]

Author: Markus Böck
Date: 2022-07-19T10:58:25+02:00
New Revision: 61394636f099ebba3d3dc7e3382a1e63f751b911

URL: https://github.com/llvm/llvm-project/commit/61394636f099ebba3d3dc7e3382a1e63f751b911
DIFF: https://github.com/llvm/llvm-project/commit/61394636f099ebba3d3dc7e3382a1e63f751b911.diff

LOG: [mlir] Ignore effects on allocated results when checking whether the op is trivially dead.

In the current state, this is only special cased for Allocation effects, but any effects on results allocated by the operation may be ignored when checking whether the op may be removed, as none of them are possible to be observed if the result is unused.

A use case for this is for IRs for languages which always initialize on allocation. To correctly model such operations, a Write as well as an Allocation effect should be placed on the result. This would prevent the Op from being deleted if unused however. This patch fixes that issue.

Differential Revision: https://reviews.llvm.org/D129854

Added: 
    

Modified: 
    mlir/lib/Interfaces/SideEffectInterfaces.cpp
    mlir/test/Transforms/canonicalize-dce.mlir
    mlir/test/lib/Dialect/Test/TestOps.td

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Interfaces/SideEffectInterfaces.cpp b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
index 1a1970db35228..8685ba270a63c 100644
--- a/mlir/lib/Interfaces/SideEffectInterfaces.cpp
+++ b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
@@ -8,6 +8,8 @@
 
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 
+#include "llvm/ADT/SmallPtrSet.h"
+
 using namespace mlir;
 
 //===----------------------------------------------------------------------===//
@@ -62,11 +64,20 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
       // memory.
       SmallVector<MemoryEffects::EffectInstance, 1> effects;
       effectInterface.getEffects(effects);
-      if (!llvm::all_of(effects, [op](const MemoryEffects::EffectInstance &it) {
-            // We can drop allocations if the value is a result of the
-            // operation.
-            if (isa<MemoryEffects::Allocate>(it.getEffect()))
-              return it.getValue() && it.getValue().getDefiningOp() == op;
+
+      // Gather all results of this op that are allocated.
+      SmallPtrSet<Value, 4> allocResults;
+      for (const MemoryEffects::EffectInstance &it : effects)
+        if (isa<MemoryEffects::Allocate>(it.getEffect()) && it.getValue() &&
+            it.getValue().getDefiningOp() == op)
+          allocResults.insert(it.getValue());
+
+      if (!llvm::all_of(effects, [&allocResults](
+                                     const MemoryEffects::EffectInstance &it) {
+            // We can drop effects if the value is an allocation and is a result
+            // of the operation.
+            if (allocResults.contains(it.getValue()))
+              return true;
             // Otherwise, the effect must be a read.
             return isa<MemoryEffects::Read>(it.getEffect());
           })) {

diff  --git a/mlir/test/Transforms/canonicalize-dce.mlir b/mlir/test/Transforms/canonicalize-dce.mlir
index fdab6b570e095..d118768764376 100644
--- a/mlir/test/Transforms/canonicalize-dce.mlir
+++ b/mlir/test/Transforms/canonicalize-dce.mlir
@@ -173,3 +173,17 @@ func.func @f() {
   }
   return
 }
+
+// -----
+
+
+// Test case: Delete ops that only have side-effects on an allocated result.
+
+// CHECK:      func @f()
+// CHECK-NOT:    test_effects_result
+// CHECK-NEXT:   return
+
+func.func @f() {
+  %0 = "test.test_effects_result"() : () -> i32
+  return
+}

diff  --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 08f62b00e0e0a..86c81dc883cdf 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2867,6 +2867,10 @@ def TestEffectsRead : TEST_Op<"op_with_memread",
 def TestEffectsWrite : TEST_Op<"op_with_memwrite",
     [MemoryEffects<[MemWrite]>]>;
 
+def TestEffectsResult : TEST_Op<"test_effects_result"> {
+  let results = (outs Res<I32, "", [MemAlloc, MemWrite]>);
+}
+
 //===----------------------------------------------------------------------===//
 // Test Ops with verifiers
 //===----------------------------------------------------------------------===//