[MLIR] Fix hasNoInterveningEffect in the presence of ops from different affine scopes

Fix hasNoInterveningEffect in the presence of ops from different affine
scopes. Also, correctly check for dependence failures as well instead of
just for the existence of a dependence.

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

diff --git a/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h b/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h
index 63a69e8..3f4ac7d 100644 (file)
--- a/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h
+++ b/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h
@@ -177,6 +177,12 @@ inline bool hasDependence(DependenceResult result) {
   return result.value == DependenceResult::HasDependence;
 }
 
+/// Returns true if the provided DependenceResult corresponds to the absence of
+/// a dependence.
+inline bool noDependence(DependenceResult result) {
+  return result.value == DependenceResult::NoDependence;
+}
+
 /// Returns in 'depCompsVec', dependence components for dependences between all
 /// load and store ops in loop nest rooted at 'forOp', at loop depths in range
 /// [1, maxLoopDepth].

diff --git a/mlir/lib/Dialect/Affine/Utils/Utils.cpp b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
index 47f1723..80e830c 100644 (file)
--- a/mlir/lib/Dialect/Affine/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
@@ -701,11 +701,12 @@ static bool hasNoInterveningEffect(Operation *start, T memOp) {
       if (isa<AffineReadOpInterface, AffineWriteOpInterface>(op)) {
         MemRefAccess srcAccess(op);
         MemRefAccess destAccess(memOp);
-        // Dependence analysis is only correct if both ops operate on the same
-        // memref.
-        if (srcAccess.memref == destAccess.memref) {
-          FlatAffineValueConstraints dependenceConstraints;
-
+        // Affine dependence analysis here is applicable only if both ops
+        // operate on the same memref and if `op`, `memOp`, and `start` are in
+        // the same AffineScope.
+        if (srcAccess.memref == destAccess.memref &&
+            getAffineScope(op) == getAffineScope(memOp) &&
+            getAffineScope(op) == getAffineScope(start)) {
           // Number of loops containing the start op and the ending operation.
           unsigned minSurroundingLoops =
               getNumCommonSurroundingLoops(*start, *memOp);
@@ -722,11 +723,14 @@ static bool hasNoInterveningEffect(Operation *start, T memOp) {
           // minSurrounding loops since `start` would overwrite any store with a
           // smaller number of surrounding loops before.
           unsigned d;
+          FlatAffineValueConstraints dependenceConstraints;
           for (d = nsLoops + 1; d > minSurroundingLoops; d--) {
             DependenceResult result = checkMemrefAccessDependence(
                 srcAccess, destAccess, d, &dependenceConstraints,
                 /*dependenceComponents=*/nullptr);
-            if (hasDependence(result)) {
+            // A dependence failure or the presence of a dependence implies a
+            // side effect.
+            if (!noDependence(result)) {
               hasSideEffect = true;
               return;
             }
@@ -735,7 +739,11 @@ static bool hasNoInterveningEffect(Operation *start, T memOp) {
           // No side effect was seen, simply return.
           return;
         }
+        // TODO: Check here if the memrefs alias: there is no side effect if
+        // `srcAccess.memref` and `destAccess.memref` don't alias.
       }
+      // We have an op with a memory effect and we cannot prove if it
+      // intervenes.
       hasSideEffect = true;
       return;
     }

diff --git a/mlir/test/Dialect/Affine/scalrep.mlir b/mlir/test/Dialect/Affine/scalrep.mlir
index 3bb8d10..8adf49b 100644 (file)
--- a/mlir/test/Dialect/Affine/scalrep.mlir
+++ b/mlir/test/Dialect/Affine/scalrep.mlir
@@ -765,3 +765,27 @@ func.func @affine_load_store_in_different_scopes() -> memref<1xf32> {
   // CHECK:      affine.load
   return %A : memref<1xf32>
 }
+
+// No forwarding should again happen here.
+
+// CHECK-LABEL: func.func @no_forwarding_across_scopes
+func.func @no_forwarding_across_scopes() -> memref<1xf32> {
+  %A = memref.alloc() : memref<1xf32>
+  %cf0 = arith.constant 0.0 : f32
+  %cf5 = arith.constant 5.0 : f32
+  %c0 = arith.constant 0 : index
+  %c100 = arith.constant 100 : index
+  %c1 = arith.constant 1 : index
+
+  // Store shouldn't be forwarded to the load.
+  affine.store %cf0, %A[0] : memref<1xf32>
+  // CHECK:      test.affine_scope
+  // CHECK-NEXT:   affine.load
+  test.affine_scope {
+    %l = affine.load %A[0] : memref<1xf32>
+    %s = arith.addf %l, %cf5 : f32
+    affine.store %s, %A[0] : memref<1xf32>
+    "terminator"() : () -> ()
+  }
+  return %A : memref<1xf32>
+}