[Mlir-commits] [mlir] [mlir][arith] Add `overflow<zero>` to `arith.remsi` (PR #237021)

[Federico Bruzzone]

================
@@ -447,6 +447,49 @@ class ArithOpConversion final : public OpConversionPattern<ArithOp> {
   }
 };
 
+class RemSIOpConversion final : public OpConversionPattern<arith::RemSIOp> {
+public:
+  using Base::Base;
+
+  LogicalResult
+  matchAndRewrite(arith::RemSIOp op, OpAdaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const override {
+    Type type = getTypeConverter()->convertType(op.getType());
+    if (!type)
+      return rewriter.notifyMatchFailure(op, "converting result type failed");
+    Value lhs = adaptor.getLhs();
+    Value rhs = adaptor.getRhs();
+    Type remainderType = type;
+    if (op.getIsOverflowZero()) {
+      // C/C++ '%' is undefined for INT_MIN % -1. Since x % -1 is zero,
+      // replace the dividend with zero when rhs is -1 before evaluating '%'.
+      if (!isa<IntegerType>(type) && !emitc::isPointerWideType(type))
+        return rewriter.notifyMatchFailure(op, "unsupported result type");
+      Location loc = op.getLoc();
+      remainderType =
+          adaptIntegralTypeSignedness(type, /*needsUnsigned=*/false);
+      lhs = adaptValueType(lhs, rewriter, remainderType);
+      rhs = adaptValueType(rhs, rewriter, remainderType);
----------------
FedericoBruzzone wrote:

Without `overflow<zero>`, `remsi` on index emits `%` on unsigned, so the remainder is computed incorrectly for negative operands. IDK, but maybe it should be enough to hoist these 3 lines outside the if body (?)

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