Revise the AffineExpr printing logic to be more careful about paren emission.
This is still (intentionally) generating redundant parens for nested tightly
binding expressions, but I think that is reasonable for readability sake.
This also print x-y instead of x-(y*1)
PiperOrigin-RevId: 206847212
diff --git a/lib/IR/AsmPrinter.cpp b/lib/IR/AsmPrinter.cpp
index 1102559..0bc573b 100644
--- a/lib/IR/AsmPrinter.cpp
+++ b/lib/IR/AsmPrinter.cpp
@@ -194,7 +194,15 @@
void printAffineMapId(int affineMapId) const;
void printAffineMapReference(const AffineMap *affineMap);
- void printAffineBinaryOpExpr(const AffineBinaryOpExpr *expr);
+ /// This enum is used to represent the binding stength of the enclosing
+ /// context that an AffineExpr is being printed in, so we can intelligently
+ /// produce parens.
+ enum class BindingStrength {
+ Weak, // + and -
+ Strong, // All other binary operators.
+ };
+ void printAffineExprInternal(const AffineExpr *expr,
+ BindingStrength enclosingTightness);
};
} // end anonymous namespace
@@ -364,6 +372,12 @@
//===----------------------------------------------------------------------===//
void ModulePrinter::printAffineExpr(const AffineExpr *expr) {
+ printAffineExprInternal(expr, BindingStrength::Weak);
+}
+
+void ModulePrinter::printAffineExprInternal(
+ const AffineExpr *expr, BindingStrength enclosingTightness) {
+ const char *binopSpelling = nullptr;
switch (expr->getKind()) {
case AffineExpr::Kind::SymbolId:
os << 's' << cast<AffineSymbolExpr>(expr)->getPosition();
@@ -375,53 +389,64 @@
os << cast<AffineConstantExpr>(expr)->getValue();
return;
case AffineExpr::Kind::Add:
+ binopSpelling = " + ";
+ break;
case AffineExpr::Kind::Mul:
+ binopSpelling = " * ";
+ break;
case AffineExpr::Kind::FloorDiv:
+ binopSpelling = " floordiv ";
+ break;
case AffineExpr::Kind::CeilDiv:
+ binopSpelling = " ceildiv ";
+ break;
case AffineExpr::Kind::Mod:
- return printAffineBinaryOpExpr(cast<AffineBinaryOpExpr>(expr));
+ binopSpelling = " mod ";
+ break;
}
-}
-void ModulePrinter::printAffineBinaryOpExpr(const AffineBinaryOpExpr *expr) {
- if (expr->getKind() != AffineExpr::Kind::Add) {
- os << '(';
- printAffineExpr(expr->getLHS());
- switch (expr->getKind()) {
- case AffineExpr::Kind::Mul:
- os << " * ";
- break;
- case AffineExpr::Kind::FloorDiv:
- os << " floordiv ";
- break;
- case AffineExpr::Kind::CeilDiv:
- os << " ceildiv ";
- break;
- case AffineExpr::Kind::Mod:
- os << " mod ";
- break;
- default:
- llvm_unreachable("unexpected affine binary op expression");
- }
+ auto *binOp = cast<AffineBinaryOpExpr>(expr);
- printAffineExpr(expr->getRHS());
- os << ')';
+ // Handle tightly binding binary operators.
+ if (binOp->getKind() != AffineExpr::Kind::Add) {
+ if (enclosingTightness == BindingStrength::Strong)
+ os << '(';
+
+ printAffineExprInternal(binOp->getLHS(), BindingStrength::Strong);
+ os << binopSpelling;
+ printAffineExprInternal(binOp->getRHS(), BindingStrength::Strong);
+
+ if (enclosingTightness == BindingStrength::Strong)
+ os << ')';
return;
}
// Print out special "pretty" forms for add.
- os << '(';
- printAffineExpr(expr->getLHS());
+ if (enclosingTightness == BindingStrength::Strong)
+ os << '(';
// Pretty print addition to a product that has a negative operand as a
// subtraction.
- if (auto *rhs = dyn_cast<AffineBinaryOpExpr>(expr->getRHS())) {
+ if (auto *rhs = dyn_cast<AffineBinaryOpExpr>(binOp->getRHS())) {
if (rhs->getKind() == AffineExpr::Kind::Mul) {
if (auto *rrhs = dyn_cast<AffineConstantExpr>(rhs->getRHS())) {
- if (rrhs->getValue() < 0) {
+ if (rrhs->getValue() == -1) {
+ printAffineExprInternal(binOp->getLHS(), BindingStrength::Weak);
+ os << " - ";
+ printAffineExprInternal(rhs->getLHS(), BindingStrength::Weak);
+
+ if (enclosingTightness == BindingStrength::Strong)
+ os << ')';
+ return;
+ }
+
+ if (rrhs->getValue() < -1) {
+ printAffineExprInternal(binOp->getLHS(), BindingStrength::Weak);
os << " - (";
- printAffineExpr(rhs->getLHS());
- os << " * " << -rrhs->getValue() << "))";
+ printAffineExprInternal(rhs->getLHS(), BindingStrength::Strong);
+ os << " * " << -rrhs->getValue() << ')';
+ if (enclosingTightness == BindingStrength::Strong)
+ os << ')';
return;
}
}
@@ -429,16 +454,20 @@
}
// Pretty print addition to a negative number as a subtraction.
- if (auto *rhs = dyn_cast<AffineConstantExpr>(expr->getRHS())) {
+ if (auto *rhs = dyn_cast<AffineConstantExpr>(binOp->getRHS())) {
if (rhs->getValue() < 0) {
- os << " - " << -rhs->getValue() << ")";
+ printAffineExprInternal(binOp->getLHS(), BindingStrength::Weak);
+ os << " - " << -rhs->getValue() << ')';
return;
}
}
+ printAffineExprInternal(binOp->getLHS(), BindingStrength::Weak);
os << " + ";
- printAffineExpr(expr->getRHS());
- os << ')';
+ printAffineExprInternal(binOp->getRHS(), BindingStrength::Weak);
+
+ if (enclosingTightness == BindingStrength::Strong)
+ os << ')';
}
void ModulePrinter::printAffineMap(const AffineMap *map) {
diff --git a/lib/IR/StandardOps.cpp b/lib/IR/StandardOps.cpp
index 66d3d05..3d74e58 100644
--- a/lib/IR/StandardOps.cpp
+++ b/lib/IR/StandardOps.cpp
@@ -96,11 +96,9 @@
auto *affineIntTy = builder.getAffineIntType();
AffineMapAttr *mapAttr;
- if (parser->parseAttribute(mapAttr))
- return {};
-
unsigned numDims;
- if (parseDimAndSymbolList(parser, opInfos, operands, numDims))
+ if (parser->parseAttribute(mapAttr) ||
+ parseDimAndSymbolList(parser, opInfos, operands, numDims))
return {};
auto *map = mapAttr->getValue();
@@ -158,13 +156,11 @@
SmallVector<SSAValue *, 4> operands;
SmallVector<OpAsmParser::OperandType, 4> operandsInfo;
- // Parse the dimension operands and optional symbol operands.
+ // Parse the dimension operands and optional symbol operands, followed by a
+ // memref type.
unsigned numDimOperands;
- if (parseDimAndSymbolList(parser, operandsInfo, operands, numDimOperands))
- return {};
-
- // Parse memref type.
- if (parser->parseColonType(type))
+ if (parseDimAndSymbolList(parser, operandsInfo, operands, numDimOperands) ||
+ parser->parseColonType(type))
return {};
// Check numDynamicDims against number of question marks in memref type.
diff --git a/test/IR/affine-map.mlir b/test/IR/affine-map.mlir
index c9a2d74..161883b 100644
--- a/test/IR/affine-map.mlir
+++ b/test/IR/affine-map.mlir
@@ -11,98 +11,98 @@
// All three maps are unique'd as one map and so there
// should be only one output.
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> ((d0 + 1), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0 + 1, d1)
#map3 = (i, j) -> (i+1, j)
// CHECK-EMPTY
#map3a = (i, j) -> (1+i, j)
// CHECK-EMPTY
#map3b = (i, j) -> (2+3-2*2+i, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> ((d0 + 2), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0 + 2, d1)
#map4 = (i, j) -> (3+3-2*2+i, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 + s0), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + s0, d1)
#map5 = (i, j)[s0] -> (i + s0, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 + s0), (d1 + 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + s0, d1 + 5)
#map6 = (i, j)[s0] -> (i + s0, j + 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (((d0 + d1) + s0), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + d1 + s0, d1)
#map7 = (i, j)[s0] -> (i + j + s0, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((((d0 + 5) + d1) + s0), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + 5 + d1 + s0, d1)
#map8 = (i, j)[s0] -> (5 + i + j + s0, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (((d0 + d1) + 5), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + d1 + 5, d1)
#map9 = (i, j)[s0] -> ((i + j) + 5, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 + (d1 + 5)), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + d1 + 5, d1)
#map10 = (i, j)[s0] -> (i + (j + 5), j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 * 2), (d1 * 3))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 * 2, d1 * 3)
#map11 = (i, j)[s0] -> (2*i, 3*j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (((d0 + 12) + ((d1 + (s0 * 3)) * 5)), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + 12 + (d1 + s0 * 3) * 5, d1)
#map12 = (i, j)[s0] -> (i + 2*6 + 5*(j+s0*3), j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (((d0 * 5) + d1), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 * 5 + d1, d1)
#map13 = (i, j)[s0] -> (5*i + j, j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 + d1), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + d1, d1)
#map14 = (i, j)[s0] -> ((i + j), (j))
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (((d0 + d1) + 5), (d1 + 3))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0 + d1 + 5, d1 + 3)
#map15 = (i, j)[s0] -> ((i + j)+5, (j)+3)
// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0, 0)
#map16 = (i, j)[s1] -> (i, 0)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0, (d1 * s0))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0, d1 * s0)
#map17 = (i, j)[s0] -> (i, s0*j)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0, ((d0 * 3) + d1))
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0, d0 * 3 + d1)
#map19 = (i, j) -> (i, 3*i + j)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0, (d0 + (d1 * 3)))
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0, d0 + d1 * 3)
#map20 = (i, j) -> (i, i + 3*j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0, (((d0 * ((s0 * s0) * 9)) + 2) + 1))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (d0, d0 * ((s0 * s0) * 9) + 2 + 1)
#map18 = (i, j)[N] -> (i, 2 + N*N*9*i + 1)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> (1, ((d0 + (d1 * 3)) + 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (1, d0 + d1 * 3 + 5)
#map21 = (i, j) -> (1, i + 3*j + 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((s0 * 5), ((d0 + (d1 * 3)) + (d0 * 5)))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> (s0 * 5, d0 + d1 * 3 + d0 * 5)
#map22 = (i, j)[s0] -> (5*s0, i + 3*j + 5*i)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> ((d0 * (s0 * s1)), d1)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0 * (s0 * s1), d1)
#map23 = (i, j)[s0, s1] -> (i*(s0*s1), j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, (d1 mod 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1 mod 5)
#map24 = (i, j)[s0, s1] -> (i, j mod 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, (d1 floordiv 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1 floordiv 5)
#map25 = (i, j)[s0, s1] -> (i, j floordiv 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, (d1 ceildiv 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1 ceildiv 5)
#map26 = (i, j)[s0, s1] -> (i, j ceildiv 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, ((d0 - (d1 * 1)) - 5))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d0 - d1 - 5)
#map29 = (i, j)[s0, s1] -> (i, i - j - 5)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, ((d0 - ((d1 * s1) * 1)) + 2))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d0 - d1 * s1 + 2)
#map30 = (i, j)[M, N] -> (i, i - N*j + 2)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> ((d0 * -5), (d1 * -3), -2, ((d0 + d1) * -1), (s0 * -1))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0 * -5, d1 * -3, -2, (d0 + d1) * -1, s0 * -1)
#map32 = (i, j)[s0, s1] -> (-5*i, -3*j, -2, -1*(i+j), -1*s0)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> (-4, (d0 * -1))
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (-4, d0 * -1)
#map33 = (i, j) -> (-2+-5-(-3), -1*i)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, (d1 floordiv s0), (d1 mod s0))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1 floordiv s0, d1 mod s0)
#map34 = (i, j)[s0, s1] -> (i, j floordiv s0, j mod s0)
-// CHECK: #map{{[0-9]+}} = (d0, d1, d2)[s0, s1, s2] -> (((((d0 * s1) * s2) + (d1 * s1)) + d2))
+// CHECK: #map{{[0-9]+}} = (d0, d1, d2)[s0, s1, s2] -> ((d0 * s1) * s2 + d1 * s1 + d2)
#map35 = (i, j, k)[s0, s1, s2] -> (i*s1*s2 + j*s1 + k)
// CHECK: #map{{[0-9]+}} = (d0, d1) -> (8, 4, 1, 3, 2, 4)
@@ -111,21 +111,27 @@
// CHECK: #map{{[0-9]+}} = (d0, d1) -> (4, 11, 512, 15)
#map37 = (i, j) -> (5 mod 3 + 2, 5*3 - 4, 128 * (500 ceildiv 128), 40 floordiv 7 * 3)
-// CHECK: #map{{[0-9]+}} = (d0, d1) -> (((d0 * 2) + 1), (d1 + 2))
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0 * 2 + 1, d1 + 2)
#map38 = (i, j) -> (1 + i*2, 2 + j)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> ((d0 * s0), (d0 + s0), (d0 + 2), (d1 * 2), (s1 * 2), (s0 + 2))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0 * s0, d0 + s0, d0 + 2, d1 * 2, s1 * 2, s0 + 2)
#map39 = (i, j)[M, N] -> (i*M, M + i, 2+i, j*2, N*2, 2 + M)
// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0, d1) size (10, 20)
#map40 = (i, j) -> (i, j) size (10, 20)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1) size (s0, (s1 + 10))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1) size (s0, s1 + 10)
#map41 = (i, j)[N, M] -> (i, j) size (N, M+10)
-// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1) size (128, (((s0 * 2) + 5) + s1))
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0, s1] -> (d0, d1) size (128, s0 * 2 + 5 + s1)
#map42 = (i, j)[N, M] -> (i, j) size (64 + 64, 5 + 2*N + M)
+// CHECK: #map{{[0-9]+}} = (d0, d1)[s0] -> ((d0 * 5) floordiv 4, (d1 ceildiv 7) mod s0)
+#map43 = (i, j) [s0] -> ( i * 5 floordiv 4, j ceildiv 7 mod s0)
+
+// CHECK: #map{{[0-9]+}} = (d0, d1) -> (d0 - d1 * 2)
+#map44 = (i, j) -> (i - 2*j)
+
// CHECK: extfunc @f0(memref<2x4xi8, #map{{[0-9]+}}, 1>)
extfunc @f0(memref<2x4xi8, #map0, 1>)
@@ -249,4 +255,9 @@
// CHECK: extfunc @f43(memref<2x4xi8, #map{{[0-9]+}}>)
extfunc @f43(memref<2x4xi8, #map42>)
+// CHECK: extfunc @f44(memref<2x4xi8, #map{{[0-9]+}}>)
+extfunc @f44(memref<2x4xi8, #map43>)
+
+// CHECK: extfunc @f45(memref<2xi8, #map{{[0-9]+}}>)
+extfunc @f45(memref<2xi8, #map44>)
diff --git a/test/IR/core-ops.mlir b/test/IR/core-ops.mlir
index aa625dd..6af4cc6 100644
--- a/test/IR/core-ops.mlir
+++ b/test/IR/core-ops.mlir
@@ -1,12 +1,12 @@
// RUN: %S/../../mlir-opt %s -o - | FileCheck %s
-// CHECK: #map0 = (d0) -> ((d0 + 1))
+// CHECK: #map0 = (d0) -> (d0 + 1)
-// CHECK: #map1 = (d0, d1) -> ((d0 + 1), (d1 + 2))
+// CHECK: #map1 = (d0, d1) -> (d0 + 1, d1 + 2)
#map5 = (d0, d1) -> (d0 + 1, d1 + 2)
-// CHECK: #map2 = (d0, d1)[s0, s1] -> ((d0 + s1), (d1 + s0))
-// CHECK: #map3 = ()[s0] -> ((s0 + 1))
+// CHECK: #map2 = (d0, d1)[s0, s1] -> (d0 + s1, d1 + s0)
+// CHECK: #map3 = ()[s0] -> (s0 + 1)
// CHECK-LABEL: cfgfunc @cfgfunc_with_ops(f32) {
cfgfunc @cfgfunc_with_ops(f32) {
diff --git a/test/IR/memory-ops.mlir b/test/IR/memory-ops.mlir
index b435116..5702632 100644
--- a/test/IR/memory-ops.mlir
+++ b/test/IR/memory-ops.mlir
@@ -15,12 +15,12 @@
%3 = alloc(%1, %2) : memref<?x?xf32, (d0, d1) -> (d0, d1), 1>
// Test alloc with no dynamic dimensions and one symbol.
- // CHECK: %4 = alloc()[%1] : memref<2x4xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1>
+ // CHECK: %4 = alloc()[%1] : memref<2x4xf32, (d0, d1)[s0] -> (d0 + s0, d1), 1>
%4 = alloc()[%1] : memref<2x4xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1>
// Test alloc with dynamic dimensions and one symbol.
- // CHECK: %5 = alloc(%2)[%1] : memref<2x?xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1>
- %5 = alloc(%2)[%1] : memref<2x?xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1>
+ // CHECK: %5 = alloc(%2)[%1] : memref<2x?xf32, (d0, d1)[s0] -> (d0 + s0, d1), 1>
+ %5 = alloc(%2)[%1] : memref<2x?xf32, (d0, d1)[s0] -> (d0 + s0, d1), 1>
// CHECK: return
return