diff --git a/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h b/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h --- a/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h +++ b/mlir/include/mlir/Dialect/Affine/Analysis/AffineAnalysis.h @@ -177,6 +177,12 @@ 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 --- a/mlir/lib/Dialect/Affine/Utils/Utils.cpp +++ b/mlir/lib/Dialect/Affine/Utils/Utils.cpp @@ -701,11 +701,12 @@ if (isa(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 @@ // 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 @@ // 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 --- a/mlir/test/Dialect/Affine/scalrep.mlir +++ b/mlir/test/Dialect/Affine/scalrep.mlir @@ -765,3 +765,27 @@ // 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> +}