Skip to content

Commit b0f3633

Browse files
[mlir][Arith] Specify evaluation order of getExpr
The C++ standard does not specify an evaluation order for addition/... operands. E.g., in `a() + b()`, the compiler is free to evaluate `a` or `b` first. This lead to different `mlir-opt` outputs in #85895. (FileCheck passed when compiled with LLVM but failed when compiled with gcc.)
1 parent c8f3d21 commit b0f3633

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,15 @@ struct AddIOpInterface
2424
auto addIOp = cast<AddIOp>(op);
2525
assert(value == addIOp.getResult() && "invalid value");
2626

27-
cstr.bound(value) ==
28-
cstr.getExpr(addIOp.getLhs()) + cstr.getExpr(addIOp.getRhs());
27+
// Note: `getExpr` has a side effect: it may add a new column to the
28+
// constraint system. The evaluation order of addition operands is
29+
// unspecified in C++. To make sure that all compilers produce the exact
30+
// same results (that can be FileCheck'd), it is important that `getExpr`
31+
// is called first and assigned to temporary variables, and the addition
32+
// is performed afterwards.
33+
AffineExpr lhs = cstr.getExpr(addIOp.getLhs());
34+
AffineExpr rhs = cstr.getExpr(addIOp.getRhs());
35+
cstr.bound(value) == lhs + rhs;
2936
}
3037
};
3138

@@ -49,8 +56,9 @@ struct SubIOpInterface
4956
auto subIOp = cast<SubIOp>(op);
5057
assert(value == subIOp.getResult() && "invalid value");
5158

52-
cstr.bound(value) ==
53-
cstr.getExpr(subIOp.getLhs()) - cstr.getExpr(subIOp.getRhs());
59+
AffineExpr lhs = cstr.getExpr(subIOp.getLhs());
60+
AffineExpr rhs = cstr.getExpr(subIOp.getRhs());
61+
cstr.bound(value) == lhs - rhs;
5462
}
5563
};
5664

@@ -61,8 +69,9 @@ struct MulIOpInterface
6169
auto mulIOp = cast<MulIOp>(op);
6270
assert(value == mulIOp.getResult() && "invalid value");
6371

64-
cstr.bound(value) ==
65-
cstr.getExpr(mulIOp.getLhs()) * cstr.getExpr(mulIOp.getRhs());
72+
AffineExpr lhs = cstr.getExpr(mulIOp.getLhs());
73+
AffineExpr rhs = cstr.getExpr(mulIOp.getRhs());
74+
cstr.bound(value) == lhs *rhs;
6675
}
6776
};
6877

0 commit comments

Comments
 (0)