[Mlir-commits] [mlir] [mlir][Arith] Specify evaluation order of `getExpr` (PR #87859)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

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.)

---
Full diff: https://github.com/llvm/llvm-project/pull/87859.diff


1 Files Affected:

- (modified) mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp (+15-6) 


``````````diff
diff --git a/mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp b/mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp
index 9c6b50e767ea26..90895e381c74b5 100644
--- a/mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ValueBoundsOpInterfaceImpl.cpp
@@ -24,8 +24,15 @@ struct AddIOpInterface
     auto addIOp = cast<AddIOp>(op);
     assert(value == addIOp.getResult() && "invalid value");
 
-    cstr.bound(value) ==
-        cstr.getExpr(addIOp.getLhs()) + cstr.getExpr(addIOp.getRhs());
+    // Note: `getExpr` has a side effect: it may add a new column to the
+    // constraint system. The evaluation order of addition operands is
+    // unspecified in C++. To make sure that all compilers produce the exact
+    // same results (that can be FileCheck'd), it is important that `getExpr`
+    // is called first and assigned to temporary variables, and the addition
+    // is performed afterwards.
+    AffineExpr lhs = cstr.getExpr(addIOp.getLhs());
+    AffineExpr rhs = cstr.getExpr(addIOp.getRhs());
+    cstr.bound(value) == lhs + rhs;
   }
 };
 
@@ -49,8 +56,9 @@ struct SubIOpInterface
     auto subIOp = cast<SubIOp>(op);
     assert(value == subIOp.getResult() && "invalid value");
 
-    cstr.bound(value) ==
-        cstr.getExpr(subIOp.getLhs()) - cstr.getExpr(subIOp.getRhs());
+    AffineExpr lhs = cstr.getExpr(subIOp.getLhs());
+    AffineExpr rhs = cstr.getExpr(subIOp.getRhs());
+    cstr.bound(value) == lhs - rhs;
   }
 };
 
@@ -61,8 +69,9 @@ struct MulIOpInterface
     auto mulIOp = cast<MulIOp>(op);
     assert(value == mulIOp.getResult() && "invalid value");
 
-    cstr.bound(value) ==
-        cstr.getExpr(mulIOp.getLhs()) * cstr.getExpr(mulIOp.getRhs());
+    AffineExpr lhs = cstr.getExpr(mulIOp.getLhs());
+    AffineExpr rhs = cstr.getExpr(mulIOp.getRhs());
+    cstr.bound(value) == lhs *rhs;
   }
 };
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/87859