[Mlir-commits] [mlir] 54e5600 - [mlir] fix wrong symbol order in AffineApplyNormalizer
Alex Zinenko
llvmlistbot at llvm.org
Thu Feb 27 06:18:07 PST 2020
Author: Alex Zinenko
Date: 2020-02-27T15:15:29+01:00
New Revision: 54e5600e4d28535d4296a6b6baeefccc9cc9d904
URL: https://github.com/llvm/llvm-project/commit/54e5600e4d28535d4296a6b6baeefccc9cc9d904
DIFF: https://github.com/llvm/llvm-project/commit/54e5600e4d28535d4296a6b6baeefccc9cc9d904.diff
LOG: [mlir] fix wrong symbol order in AffineApplyNormalizer
Summary:
AffineApplyNormalizer provides common logic for folding affine maps that appear
in affine.apply into other affine operations that use the result of said
affine.apply. In the process, affine maps of both operations are composed.
During the composition `A.compose(B)` the symbols from the map A are placed
before those of the map B in a single concatenated symbol list. However,
AffineApplyNormalizer was ordering the operands of the operation being
normalized by iteratively appending the symbols into a single list accoridng to
the operand order, regardless of whether these operands are symbols of the
current operation or of the map that is being folded into it. This could lead
to wrong order of symbols and, when the symbols were bound to constant values,
to visibly incorrect folding of constants into affine maps as reported in
PR45031. Make sure symbols operands to the current operation are always placed
before symbols coming from the folded maps.
Update the test that was exercising the incorrect folder behavior. For some
reason, the order of symbol operands was swapped in the test input compared to
the previous operations, making it easy to assume the correct maps were
produced whereas they were swapping the symbols back due to the problem
described above.
Closes https://bugs.llvm.org/show_bug.cgi?id=45031
Differential Revision: https://reviews.llvm.org/D75247
Added:
Modified:
mlir/include/mlir/Dialect/AffineOps/AffineOps.h
mlir/lib/Dialect/AffineOps/AffineOps.cpp
mlir/test/Dialect/AffineOps/canonicalize.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/AffineOps/AffineOps.h b/mlir/include/mlir/Dialect/AffineOps/AffineOps.h
index 76bb14ea97c8..6c157e2a68ce 100644
--- a/mlir/include/mlir/Dialect/AffineOps/AffineOps.h
+++ b/mlir/include/mlir/Dialect/AffineOps/AffineOps.h
@@ -654,6 +654,10 @@ struct AffineApplyNormalizer {
SmallVector<Value, 8> reorderedDims;
SmallVector<Value, 8> concatenatedSymbols;
+ /// The number of symbols in concatenated symbols that belong to the original
+ /// map as opposed to those concatendated during map composition.
+ unsigned numProperSymbols;
+
AffineMap affineMap;
/// Used with RAII to control the depth at which AffineApply are composed
@@ -667,7 +671,7 @@ struct AffineApplyNormalizer {
}
static constexpr unsigned kMaxAffineApplyDepth = 1;
- AffineApplyNormalizer() { affineApplyDepth()++; }
+ AffineApplyNormalizer() : numProperSymbols(0) { affineApplyDepth()++; }
public:
~AffineApplyNormalizer() { affineApplyDepth()--; }
diff --git a/mlir/lib/Dialect/AffineOps/AffineOps.cpp b/mlir/lib/Dialect/AffineOps/AffineOps.cpp
index 4133b669df75..8acc158352a3 100644
--- a/mlir/lib/Dialect/AffineOps/AffineOps.cpp
+++ b/mlir/lib/Dialect/AffineOps/AffineOps.cpp
@@ -536,8 +536,12 @@ AffineApplyNormalizer::AffineApplyNormalizer(AffineMap map,
auxiliaryExprs.push_back(renumberOneDim(t));
} else {
// c. The mathematical composition of AffineMap concatenates symbols.
- // We do the same for symbol operands.
- concatenatedSymbols.push_back(t);
+ // Note that the map composition will put symbols already present
+ // in the map before any symbols coming from the auxiliary map, so
+ // we insert them before any symbols that are due to renumbering,
+ // and after the proper symbols we have seen already.
+ concatenatedSymbols.insert(
+ std::next(concatenatedSymbols.begin(), numProperSymbols++), t);
}
}
}
diff --git a/mlir/test/Dialect/AffineOps/canonicalize.mlir b/mlir/test/Dialect/AffineOps/canonicalize.mlir
index be8d000091ad..26220925bfb3 100644
--- a/mlir/test/Dialect/AffineOps/canonicalize.mlir
+++ b/mlir/test/Dialect/AffineOps/canonicalize.mlir
@@ -17,7 +17,7 @@
// Affine maps for test case: compose_affine_maps_dependent_loads
// CHECK-DAG: [[MAP9:#map[0-9]+]] = affine_map<(d0) -> (d0 + 3)>
// CHECK-DAG: [[MAP10:#map[0-9]+]] = affine_map<(d0) -> (d0 * 3)>
-// CHECK-DAG: [[MAP11:#map[0-9]+]] = affine_map<(d0) -> ((d0 + 7) ceildiv 3)>
+// CHECK-DAG: [[MAP11:#map[0-9]+]] = affine_map<(d0) -> ((d0 + 3) ceildiv 3)>
// CHECK-DAG: [[MAP12:#map[0-9]+]] = affine_map<(d0) -> (d0 * 7 - 49)>
// Affine maps for test case: compose_affine_maps_diamond_dependency
@@ -187,9 +187,9 @@ func @compose_affine_maps_dependent_loads() {
// Swizzle %x00, %x01 and %c3, %c7
%x10 = affine.apply affine_map<(d0, d1)[s0, s1] -> (d0 * s1)>
- (%x01, %x00)[%c7, %c3]
+ (%x01, %x00)[%c3, %c7]
%x11 = affine.apply affine_map<(d0, d1)[s0, s1] -> (d1 ceildiv s0)>
- (%x01, %x00)[%c7, %c3]
+ (%x01, %x00)[%c3, %c7]
// CHECK-NEXT: [[I2A:%[0-9]+]] = affine.apply [[MAP12]](%{{.*}})
// CHECK-NEXT: [[I2B:%[0-9]+]] = affine.apply [[MAP11]](%{{.*}})
@@ -568,3 +568,29 @@ func @affine_min(%arg0: index) {
}
return
}
+
+// -----
+
+// Reproducer for PR45031. This used to fold into an incorrect map because
+// symbols were concatenated in the wrong order during map folding. Map
+// composition places the symbols of the original map before those of the map
+// it is composed with, e.g. A.compose(B) will first have all symbols of A,
+// then all symbols of B.
+
+#map1 = affine_map<(d0)[s0, s1] -> (d0 * s0 + s1)>
+#map2 = affine_map<(d0)[s0] -> (1024, -d0 + s0)>
+
+// CHECK: #[[MAP:.*]] = affine_map<()[s0, s1] -> (1024, s1 * -1024 + s0)>
+
+// CHECK: func @rep(%[[ARG0:.*]]: index, %[[ARG1:.*]]: index)
+func @rep(%arg0 : index, %arg1 : index) -> index {
+ // CHECK-NOT: constant
+ %c0 = constant 0 : index
+ %c1024 = constant 1024 : index
+ // CHECK-NOT: affine.apply
+ %0 = affine.apply #map1(%arg0)[%c1024, %c0]
+
+ // CHECK: affine.min #[[MAP]]()[%[[ARG1]], %[[ARG0]]]
+ %1 = affine.min #map2(%0)[%arg1]
+ return %1 : index
+}
More information about the Mlir-commits
mailing list