[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