Skip to content

Commit 6eab083

Browse files
authored
[mlir][emitc] Refactor brackets in expressions (llvm#168267)
This patch is a minor NFC-intended refactoring to the way emitting redundant parentheses is prevented. The current implementation pushes and later pops a fake low precedence into the precedence stack when emitting function calls. The new implementation adds a boolean argument to `emitOperand()` that explicity guarantees that the operand is being emitted between some kind of brackets, exempting the method from enforcing correct evaluation order w.r.t precedence and associativity up the expression tree.
1 parent b6fd3c6 commit 6eab083

File tree

1 file changed

+15
-16
lines changed

1 file changed

+15
-16
lines changed

mlir/lib/Target/Cpp/TranslateToCpp.cpp

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,11 @@ struct CppEmitter {
173173
/// Emits the operands of the operation. All operands are emitted in order.
174174
LogicalResult emitOperands(Operation &op);
175175

176-
/// Emits value as an operands of an operation
177-
LogicalResult emitOperand(Value value);
176+
/// Emits value as an operand of some operation. Unless \p isInBrackets is
177+
/// true, operands emitted as sub-expressions will be parenthesized if needed
178+
/// in order to enforce correct evaluation based on precedence and
179+
/// associativity.
180+
LogicalResult emitOperand(Value value, bool isInBrackets = false);
178181

179182
/// Emit an expression as a C expression.
180183
LogicalResult emitExpression(ExpressionOp expressionOp);
@@ -1578,18 +1581,20 @@ LogicalResult CppEmitter::emitExpression(ExpressionOp expressionOp) {
15781581
return success();
15791582
}
15801583

1581-
LogicalResult CppEmitter::emitOperand(Value value) {
1584+
LogicalResult CppEmitter::emitOperand(Value value, bool isInBrackets) {
15821585
if (isPartOfCurrentExpression(value)) {
15831586
Operation *def = value.getDefiningOp();
15841587
assert(def && "Expected operand to be defined by an operation");
15851588
FailureOr<int> precedence = getOperatorPrecedence(def);
15861589
if (failed(precedence))
15871590
return failure();
15881591

1589-
// Sub-expressions with equal or lower precedence need to be parenthesized,
1590-
// as they might be evaluated in the wrong order depending on the shape of
1591-
// the expression tree.
1592-
bool encloseInParenthesis = precedence.value() <= getExpressionPrecedence();
1592+
// Unless already in brackets, sub-expressions with equal or lower
1593+
// precedence need to be parenthesized as they might be evaluated in the
1594+
// wrong order depending on the shape of the expression tree.
1595+
bool encloseInParenthesis =
1596+
!isInBrackets && precedence.value() <= getExpressionPrecedence();
1597+
15931598
if (encloseInParenthesis)
15941599
os << "(";
15951600
pushExpressionPrecedence(precedence.value());
@@ -1628,15 +1633,9 @@ LogicalResult CppEmitter::emitOperand(Value value) {
16281633

16291634
LogicalResult CppEmitter::emitOperands(Operation &op) {
16301635
return interleaveCommaWithError(op.getOperands(), os, [&](Value operand) {
1631-
// If an expression is being emitted, push lowest precedence as these
1632-
// operands are either wrapped by parenthesis.
1633-
if (getEmittedExpression())
1634-
pushExpressionPrecedence(lowestPrecedence());
1635-
if (failed(emitOperand(operand)))
1636-
return failure();
1637-
if (getEmittedExpression())
1638-
popExpressionPrecedence();
1639-
return success();
1636+
// Emit operand under guarantee that if it's part of an expression then it
1637+
// is being emitted within brackets.
1638+
return emitOperand(operand, /*isInBrackets=*/true);
16401639
});
16411640
}
16421641

0 commit comments

Comments
 (0)