[Mlir-commits] [mlir] [mlir][arith] Fix canon pattern for large ints in chained arith (PR #68900)

Markus Böck llvmlistbot at llvm.org
Thu Oct 12 11:53:32 PDT 2023


================
@@ -39,26 +39,35 @@ using namespace mlir::arith;
 static IntegerAttr
 applyToIntegerAttrs(PatternRewriter &builder, Value res, Attribute lhs,
                     Attribute rhs,
-                    function_ref<int64_t(int64_t, int64_t)> binFn) {
-  return builder.getIntegerAttr(res.getType(),
-                                binFn(llvm::cast<IntegerAttr>(lhs).getInt(),
-                                      llvm::cast<IntegerAttr>(rhs).getInt()));
+                    function_ref<APInt(APInt, APInt&)> binFn) {
+  auto lhsVal = llvm::cast<IntegerAttr>(lhs).getValue();
+  auto rhsVal = llvm::cast<IntegerAttr>(rhs).getValue();
+  auto value = binFn(lhsVal, rhsVal);
+  return IntegerAttr::get(res.getType(), value);
 }
 
 static IntegerAttr addIntegerAttrs(PatternRewriter &builder, Value res,
                                    Attribute lhs, Attribute rhs) {
-  return applyToIntegerAttrs(builder, res, lhs, rhs, std::plus<int64_t>());
+  auto binFn = [](APInt a, APInt& b) -> APInt {
+    return std::move(a) + b;
----------------
zero9178 wrote:

```suggestion
    return a + b;
```
the `std::move` can be removed once using `const APInt&`. This is actually likely to be cheaper (or more likely identical in performance) as both your current version and the `const APInt&` version create exactly one copy. Your version does one move construction however, whereas the `const APInt&` version does not. 

You can also continue using `std::plus` in that case! Ditto the other occurences.

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


More information about the Mlir-commits mailing list