[Mlir-commits] [mlir] 28a7dfa - [MLIR] Fixed missing constraint append when adding an AffineIfOp domain

Uday Bondhugula llvmlistbot at llvm.org
Thu Aug 27 12:04:56 PDT 2020


Author: Vincent Zhao
Date: 2020-08-28T00:34:23+05:30
New Revision: 28a7dfa33d979e5ff3ed2d975c71b08d611fe6b6

URL: https://github.com/llvm/llvm-project/commit/28a7dfa33d979e5ff3ed2d975c71b08d611fe6b6
DIFF: https://github.com/llvm/llvm-project/commit/28a7dfa33d979e5ff3ed2d975c71b08d611fe6b6.diff

LOG: [MLIR] Fixed missing constraint append when adding an AffineIfOp domain

The prior diff that introduced `addAffineIfOpDomain` missed appending
constraints from the ifOp domain. This revision fixes this problem.

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

Added: 
    

Modified: 
    mlir/lib/Analysis/AffineStructures.cpp
    mlir/test/Transforms/memref-dependence-check.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Analysis/AffineStructures.cpp b/mlir/lib/Analysis/AffineStructures.cpp
index b712b2cef45e..546dfa4ba7db 100644
--- a/mlir/lib/Analysis/AffineStructures.cpp
+++ b/mlir/lib/Analysis/AffineStructures.cpp
@@ -731,8 +731,11 @@ void FlatAffineConstraints::addAffineIfOpDomain(AffineIfOp ifOp) {
   SmallVector<Value, 4> operands = ifOp.getOperands();
   cst.setIdValues(0, cst.getNumDimAndSymbolIds(), operands);
 
-  // Merge the constraints from ifOp to the current domain.
+  // Merge the constraints from ifOp to the current domain. We need first merge
+  // and align the IDs from both constraints, and then append the constraints
+  // from the ifOp into the current one.
   mergeAndAlignIdsWithOther(0, &cst);
+  append(cst);
 }
 
 // Searches for a constraint with a non-zero coefficient at 'colIdx' in

diff  --git a/mlir/test/Transforms/memref-dependence-check.mlir b/mlir/test/Transforms/memref-dependence-check.mlir
index fc1c72cf1664..fbe0a149e9f4 100644
--- a/mlir/test/Transforms/memref-dependence-check.mlir
+++ b/mlir/test/Transforms/memref-dependence-check.mlir
@@ -9,15 +9,15 @@ func @store_may_execute_before_load() {
   %m = alloc() : memref<10xf32>
   %cf7 = constant 7.0 : f32
   %c0 = constant 4 : index
-  // There is a dependence from store 0 to load 1 at depth 1 because the
-  // ancestor IfOp of the store, dominates the ancestor ForSmt of the load,
-  // and thus the store "may" conditionally execute before the load.
+  // There is no dependence from store 0 to load 1 at depth if we take into account
+  // the constraint introduced by the following `affine.if`, which indicates that
+  // the store 0 will never be executed.
   affine.if #set0(%c0) {
     affine.for %i0 = 0 to 10 {
       affine.store %cf7, %m[%i0] : memref<10xf32>
       // expected-remark at above {{dependence from 0 to 0 at depth 1 = false}}
       // expected-remark at above {{dependence from 0 to 0 at depth 2 = false}}
-      // expected-remark at above {{dependence from 0 to 1 at depth 1 = true}}
+      // expected-remark at above {{dependence from 0 to 1 at depth 1 = false}}
     }
   }
   affine.for %i1 = 0 to 10 {
@@ -1044,7 +1044,7 @@ func @test_interleaved_affine_for_if() {
   %N = dim %0, %c0 : memref<101xf32>
   %cf7 = constant 7.0 : f32
 
-  affine.for %i0 = 0 to 100 {
+  affine.for %i0 = 0 to 101 {
     affine.if #set1(%i0)[%N] {
       %1 = affine.load %0[%i0] : memref<101xf32>
       // expected-remark at above {{dependence from 0 to 0 at depth 1 = false}}


        


More information about the Mlir-commits mailing list