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

[llvmlistbot at llvm.org]

Author: Matthias Springer
Date: 2024-04-06T12:43:26+09:00
New Revision: 08200fa3f50ba9dc094467d6e1d31197dfcb7134

URL: https://github.com/llvm/llvm-project/commit/08200fa3f50ba9dc094467d6e1d31197dfcb7134
DIFF: https://github.com/llvm/llvm-project/commit/08200fa3f50ba9dc094467d6e1d31197dfcb7134.diff

LOG: [mlir][Arith] Specify evaluation order of `getExpr` (#87859)

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

Added: 
    

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

Removed: 
    


################################################################################
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;
   }
 };