[mlir][emitc] Arith to EmitC: Handle addi, subi and muli #86120
Conversation
No handling yet for divsi and divui, as they require special considerations for signedness.
|
@llvm/pr-subscribers-mlir Author: Matthias Gehre (mgehre-amd) ChangesNo handling yet for divsi and divui, as they require special considerations for signedness. Full diff: https://github.com/llvm/llvm-project/pull/86120.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
index 3532785c31b939..e85bb0f6b227b9 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
@@ -96,6 +96,9 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter,
ArithOpConversion<arith::DivFOp, emitc::DivOp>,
ArithOpConversion<arith::MulFOp, emitc::MulOp>,
ArithOpConversion<arith::SubFOp, emitc::SubOp>,
+ ArithOpConversion<arith::AddIOp, emitc::AddOp>,
+ ArithOpConversion<arith::MulIOp, emitc::MulOp>,
+ ArithOpConversion<arith::SubIOp, emitc::SubOp>,
SelectOpConversion
>(typeConverter, ctx);
// clang-format on
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
index 022530ef4db84b..e5f2c330b851c3 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
@@ -37,6 +37,19 @@ func.func @arith_ops(%arg0: f32, %arg1: f32) {
// -----
+func.func @arith_integer_ops(%arg0: i32, %arg1: i32) {
+ // CHECK: emitc.add %arg0, %arg1 : (i32, i32) -> i32
+ %0 = arith.addi %arg0, %arg1 : i32
+ // CHECK: emitc.sub %arg0, %arg1 : (i32, i32) -> i32
+ %1 = arith.subi %arg0, %arg1 : i32
+ // CHECK: emitc.mul %arg0, %arg1 : (i32, i32) -> i32
+ %2 = arith.muli %arg0, %arg1 : i32
+
+ return
+}
+
+// -----
+
func.func @arith_select(%arg0: i1, %arg1: tensor<8xi32>, %arg2: tensor<8xi32>) -> () {
// CHECK: [[V0:[^ ]*]] = emitc.conditional %arg0, %arg1, %arg2 : tensor<8xi32>
%0 = arith.select %arg0, %arg1, %arg2 : i1, tensor<8xi32>
|
There was a problem hiding this comment.
No handling yet for divsi and divui, as they require special considerations for signedness.
Probably we already need some special considerations for this case. E.g. for arith.addi the docs https://mlir.llvm.org/docs/Dialects/ArithOps/#arithaddi-arithaddiop state
Performs N-bit addition on the operands. The operands are interpreted as unsigned bitvectors. The result is represented by a bitvector containing the mathematical value of the addition modulo 2^n, where n is the bitwidth.
Following https://en.cppreference.com/w/cpp/language/operator_arithmetic#Overflows
Unsigned integer arithmetic is always performed modulo 2n where n is the number of bits in that particular integer.
we should probably cast to unsigned, perform the arithmetic operation and add another cast. WDYT?
Good point, this will otherwise cause UB for when the C types are signed. |
mgehre-amd
force-pushed
the
mgehre.emitc_addi
branch
from
ec3e3ae to
25d539d
Compare
Important to consider that `arith` has wrap around semantics, and in C++ signed overflow is UB. Unless the operation guarantees that no signed overflow happens, we will perform the arithmetic in an equivalent unsigned type. `bool` also doesn't wrap around in C++, and is not addressed here.
Important to consider that `arith` has wrap around semantics, and in C++ signed overflow is UB. Unless the operation guarantees that no signed overflow happens, we will perform the arithmetic in an equivalent unsigned type. `bool` also doesn't wrap around in C++, and is not addressed here.
Important to consider that
arithhas wrap around semantics, and in C++ signed overflow is UB.Unless the operation guarantees that no signed overflow happens, we will perform the arithmetic in an equivalent unsigned type.