[Mlir-commits] [mlir] [mlir][ArithToLLVM] Preserve defined signed remainder by -1 (PR #233161)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sat Oct 17 19:28:15 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: Matthias Springer (matthias-springer)
<details>
<summary>Changes</summary>
Select a zero dividend when the divisor is -1 before emitting `llvm.srem`. This preserves `arith.remsi(INT_MIN, -1) == 0` without introducing LLVM UB. In LLVM, `srem(INT_MIN, -1)` is undefined behavior. Presumably because some architectures have instructions for computing division and remainder at the same time.
---
Full diff: https://github.com/llvm/llvm-project/pull/233161.diff
2 Files Affected:
- (modified) mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp (+57-2)
- (modified) mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir (+85-11)
``````````diff
diff --git a/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp b/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
index 9b523a52c09a35..92df636636ee0d 100644
--- a/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
+++ b/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
@@ -185,8 +185,6 @@ using RemFOpLowering =
VectorConvertToLLVMPattern<arith::RemFOp, LLVM::FRemOp,
arith::AttrConvertFastMathToLLVM,
/*FailOnUnsupportedFP=*/true>;
-using RemSIOpLowering =
- VectorConvertToLLVMPattern<arith::RemSIOp, LLVM::SRemOp>;
using RemUIOpLowering =
VectorConvertToLLVMPattern<arith::RemUIOp, LLVM::URemOp>;
using SelectOpLowering =
@@ -293,6 +291,14 @@ using MulSIExtendedOpLowering =
using MulUIExtendedOpLowering =
MulIExtendedOpLowering<arith::MulUIExtendedOp, false>;
+struct RemSIOpLowering : public ConvertOpToLLVMPattern<arith::RemSIOp> {
+ using ConvertOpToLLVMPattern::ConvertOpToLLVMPattern;
+
+ LogicalResult
+ matchAndRewrite(arith::RemSIOp op, OpAdaptor adaptor,
+ ConversionPatternRewriter &rewriter) const override;
+};
+
struct CmpIOpLowering : public ConvertOpToLLVMPattern<arith::CmpIOp> {
using ConvertOpToLLVMPattern::ConvertOpToLLVMPattern;
@@ -692,6 +698,55 @@ LogicalResult MulIExtendedOpLowering<ArithMulOp, IsSigned>::matchAndRewrite(
"ND vector types are not supported yet");
}
+//===----------------------------------------------------------------------===//
+// RemSIOpLowering
+//===----------------------------------------------------------------------===//
+
+LogicalResult
+RemSIOpLowering::matchAndRewrite(arith::RemSIOp op, OpAdaptor adaptor,
+ ConversionPatternRewriter &rewriter) const {
+ Type operandType = adaptor.getLhs().getType();
+ if (!LLVM::isCompatibleType(operandType))
+ return failure();
+
+ Location loc = op.getLoc();
+ auto buildRemainder = [&](Type type, ValueRange operands) -> Value {
+ OpAdaptor adaptor(operands);
+ auto elementType = cast<IntegerType>(getElementTypeOrSelf(type));
+ TypedAttr minusOneAttr = rewriter.getIntegerAttr(
+ elementType, APInt::getAllOnes(elementType.getWidth()));
+ if (auto vectorType = dyn_cast<VectorType>(type))
+ minusOneAttr = SplatElementsAttr::get(vectorType, minusOneAttr);
+ Value minusOne = LLVM::ConstantOp::create(rewriter, loc, minusOneAttr);
+ Value zero =
+ LLVM::ConstantOp::create(rewriter, loc, rewriter.getZeroAttr(type));
+
+ // arith.remsi(INT_MIN, -1) is zero, but llvm.srem has undefined behavior
+ // for this pair. Select a zero dividend before taking the remainder;
+ // selecting the result would not prevent the undefined behavior.
+ Value isMinusOne = LLVM::ICmpOp::create(
+ rewriter, loc, LLVM::ICmpPredicate::eq, adaptor.getRhs(), minusOne);
+ Value lhs = LLVM::SelectOp::create(rewriter, loc, isMinusOne, zero,
+ adaptor.getLhs());
+ auto remainder = LLVM::SRemOp::create(rewriter, loc, lhs, adaptor.getRhs());
+ remainder->setDiscardableAttrs(op->getDiscardableAttrDictionary());
+ return remainder;
+ };
+
+ // Handle the scalar and 1D vector cases.
+ if (!isa<LLVM::LLVMArrayType>(operandType)) {
+ rewriter.replaceOp(op, buildRemainder(operandType, adaptor.getOperands()));
+ return success();
+ }
+
+ if (!isa<VectorType>(op.getType()))
+ return rewriter.notifyMatchFailure(op, "expected vector result type");
+
+ return LLVM::detail::handleMultidimensionalVectors(
+ op.getOperation(), adaptor.getOperands(), *getTypeConverter(),
+ buildRemainder, rewriter);
+}
+
//===----------------------------------------------------------------------===//
// CmpIOpLowering
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir b/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
index aea4be21952748..3c86de55f81631 100644
--- a/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
+++ b/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
@@ -15,25 +15,29 @@ func.func @vector_ops(%arg0: vector<4xf32>, %arg1: vector<4xi1>, %arg2: vector<4
%3 = arith.divsi %arg2, %arg3 : vector<4xi64>
// CHECK-NEXT: %3 = llvm.udiv %arg2, %arg3 : vector<4xi64>
%4 = arith.divui %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %4 = llvm.srem %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %[[MINUS_ONE:.*]] = llvm.mlir.constant(dense<-1> : vector<4xi64>) : vector<4xi64>
+// CHECK-NEXT: %[[ZERO:.*]] = llvm.mlir.constant(dense<0> : vector<4xi64>) : vector<4xi64>
+// CHECK-NEXT: %[[IS_MINUS_ONE:.*]] = llvm.icmp "eq" %arg3, %[[MINUS_ONE]] : vector<4xi64>
+// CHECK-NEXT: %[[SAFE_LHS:.*]] = llvm.select %[[IS_MINUS_ONE]], %[[ZERO]], %arg2 : vector<4xi1>, vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.srem %[[SAFE_LHS]], %arg3 : vector<4xi64>
%5 = arith.remsi %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %5 = llvm.urem %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.urem %arg2, %arg3 : vector<4xi64>
%6 = arith.remui %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %6 = llvm.fdiv %arg0, %0 : vector<4xf32>
+// CHECK-NEXT: %{{.*}} = llvm.fdiv %arg0, %0 : vector<4xf32>
%7 = arith.divf %arg0, %0 : vector<4xf32>
-// CHECK-NEXT: %7 = llvm.frem %arg0, %0 : vector<4xf32>
+// CHECK-NEXT: %{{.*}} = llvm.frem %arg0, %0 : vector<4xf32>
%8 = arith.remf %arg0, %0 : vector<4xf32>
-// CHECK-NEXT: %8 = llvm.and %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.and %arg2, %arg3 : vector<4xi64>
%9 = arith.andi %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %9 = llvm.or %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.or %arg2, %arg3 : vector<4xi64>
%10 = arith.ori %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %10 = llvm.xor %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.xor %arg2, %arg3 : vector<4xi64>
%11 = arith.xori %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %11 = llvm.shl %arg2, %arg2 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.shl %arg2, %arg2 : vector<4xi64>
%12 = arith.shli %arg2, %arg2 : vector<4xi64>
-// CHECK-NEXT: %12 = llvm.ashr %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.ashr %arg2, %arg3 : vector<4xi64>
%13 = arith.shrsi %arg2, %arg3 : vector<4xi64>
-// CHECK-NEXT: %13 = llvm.lshr %arg2, %arg3 : vector<4xi64>
+// CHECK-NEXT: %{{.*}} = llvm.lshr %arg2, %arg3 : vector<4xi64>
%14 = arith.shrui %arg2, %arg3 : vector<4xi64>
return %1 : vector<4xf32>
}
@@ -65,7 +69,11 @@ func.func @ops(f32, f32, i32, i32, f64) -> (f32, i32) {
%9 = arith.divsi %arg2, %arg3 : i32
// CHECK: = llvm.udiv %arg2, %arg3 : i32
%10 = arith.divui %arg2, %arg3 : i32
-// CHECK: = llvm.srem %arg2, %arg3 : i32
+// CHECK: %[[MINUS_ONE:.*]] = llvm.mlir.constant(-1 : i32) : i32
+// CHECK: %[[ZERO:.*]] = llvm.mlir.constant(0 : i32) : i32
+// CHECK: %[[IS_MINUS_ONE:.*]] = llvm.icmp "eq" %arg3, %[[MINUS_ONE]] : i32
+// CHECK: %[[SAFE_LHS:.*]] = llvm.select %[[IS_MINUS_ONE]], %[[ZERO]], %arg2 : i1, i32
+// CHECK: = llvm.srem %[[SAFE_LHS]], %arg3 : i32
%11 = arith.remsi %arg2, %arg3 : i32
// CHECK: = llvm.urem %arg2, %arg3 : i32
%12 = arith.remui %arg2, %arg3 : i32
@@ -95,6 +103,72 @@ func.func @ops(f32, f32, i32, i32, f64) -> (f32, i32) {
// -----
+// INT_MIN % -1 must evaluate to zero. Keep the divisor dynamic so that the
+// arith.remsi is lowered instead of being constant-folded.
+// CHECK-LABEL: @remsi_int_min(
+// CHECK-SAME: %[[RHS:.*]]: i32)
+func.func @remsi_int_min(%rhs: i32) -> i32 {
+ // CHECK-DAG: %[[MIN:.*]] = llvm.mlir.constant(-2147483648 : i32) : i32
+ // CHECK-DAG: %[[MINUS_ONE:.*]] = llvm.mlir.constant(-1 : i32) : i32
+ // CHECK-DAG: %[[ZERO:.*]] = llvm.mlir.constant(0 : i32) : i32
+ // CHECK: %[[IS_MINUS_ONE:.*]] = llvm.icmp "eq" %[[RHS]], %[[MINUS_ONE]] : i32
+ // CHECK-NEXT: %[[SAFE_LHS:.*]] = llvm.select %[[IS_MINUS_ONE]], %[[ZERO]], %[[MIN]] : i1, i32
+ // CHECK-NEXT: %[[RESULT:.*]] = llvm.srem %[[SAFE_LHS]], %[[RHS]] : i32
+ // CHECK-NEXT: return %[[RESULT]] : i32
+ %min = arith.constant -2147483648 : i32
+ %result = arith.remsi %min, %rhs : i32
+ return %result : i32
+}
+
+// -----
+
+// Replacing the divisor with 1 would not work for i1, where 1 is also -1.
+// CHECK-LABEL: @remsi_i1(
+// CHECK-SAME: %[[LHS:.*]]: i1, %[[RHS:.*]]: i1)
+func.func @remsi_i1(%lhs: i1, %rhs: i1) -> i1 {
+ // CHECK: %[[MINUS_ONE:.*]] = llvm.mlir.constant(true) : i1
+ // CHECK: %[[ZERO:.*]] = llvm.mlir.constant(false) : i1
+ // CHECK: %[[IS_MINUS_ONE:.*]] = llvm.icmp "eq" %[[RHS]], %[[MINUS_ONE]] : i1
+ // CHECK-NEXT: %[[SAFE_LHS:.*]] = llvm.select %[[IS_MINUS_ONE]], %[[ZERO]], %[[LHS]] : i1, i1
+ // CHECK-NEXT: %[[RESULT:.*]] = llvm.srem %[[SAFE_LHS]], %[[RHS]] : i1
+ // CHECK-NEXT: return %[[RESULT]] : i1
+ %result = arith.remsi %lhs, %rhs : i1
+ return %result : i1
+}
+
+// -----
+
+// Guard each subvector when lowering multidimensional vectors, including
+// scalable vectors.
+// CHECK-LABEL: @remsi_2dvector(
+// CHECK-SAME: %[[LHS:.*]]: vector<2x[4]xi32>, %[[RHS:.*]]: vector<2x[4]xi32>)
+func.func @remsi_2dvector(%lhs: vector<2x[4]xi32>, %rhs: vector<2x[4]xi32>) -> vector<2x[4]xi32> {
+ // CHECK-DAG: %[[LHS_ARRAY:.*]] = builtin.unrealized_conversion_cast %[[LHS]] : vector<2x[4]xi32> to !llvm.array<2 x vector<[4]xi32>>
+ // CHECK-DAG: %[[RHS_ARRAY:.*]] = builtin.unrealized_conversion_cast %[[RHS]] : vector<2x[4]xi32> to !llvm.array<2 x vector<[4]xi32>>
+ // CHECK: %[[LHS0:.*]] = llvm.extractvalue %[[LHS_ARRAY]][0]
+ // CHECK: %[[RHS0:.*]] = llvm.extractvalue %[[RHS_ARRAY]][0]
+ // CHECK: %[[MINUS_ONE0:.*]] = llvm.mlir.constant(dense<-1> : vector<[4]xi32>) : vector<[4]xi32>
+ // CHECK: %[[ZERO0:.*]] = llvm.mlir.constant(dense<0> : vector<[4]xi32>) : vector<[4]xi32>
+ // CHECK: %[[IS_MINUS_ONE0:.*]] = llvm.icmp "eq" %[[RHS0]], %[[MINUS_ONE0]] : vector<[4]xi32>
+ // CHECK-NEXT: %[[SAFE_LHS0:.*]] = llvm.select %[[IS_MINUS_ONE0]], %[[ZERO0]], %[[LHS0]] : vector<[4]xi1>, vector<[4]xi32>
+ // CHECK-NEXT: %[[RESULT0:.*]] = llvm.srem %[[SAFE_LHS0]], %[[RHS0]] : vector<[4]xi32>
+ // CHECK: %[[ARRAY0:.*]] = llvm.insertvalue %[[RESULT0]], %{{.*}}[0]
+ // CHECK: %[[LHS1:.*]] = llvm.extractvalue %[[LHS_ARRAY]][1]
+ // CHECK: %[[RHS1:.*]] = llvm.extractvalue %[[RHS_ARRAY]][1]
+ // CHECK: %[[MINUS_ONE1:.*]] = llvm.mlir.constant(dense<-1> : vector<[4]xi32>) : vector<[4]xi32>
+ // CHECK: %[[ZERO1:.*]] = llvm.mlir.constant(dense<0> : vector<[4]xi32>) : vector<[4]xi32>
+ // CHECK: %[[IS_MINUS_ONE1:.*]] = llvm.icmp "eq" %[[RHS1]], %[[MINUS_ONE1]] : vector<[4]xi32>
+ // CHECK-NEXT: %[[SAFE_LHS1:.*]] = llvm.select %[[IS_MINUS_ONE1]], %[[ZERO1]], %[[LHS1]] : vector<[4]xi1>, vector<[4]xi32>
+ // CHECK-NEXT: %[[RESULT1:.*]] = llvm.srem %[[SAFE_LHS1]], %[[RHS1]] : vector<[4]xi32>
+ // CHECK: %[[ARRAY1:.*]] = llvm.insertvalue %[[RESULT1]], %[[ARRAY0]][1]
+ // CHECK: %[[RESULT:.*]] = builtin.unrealized_conversion_cast %[[ARRAY1]] : !llvm.array<2 x vector<[4]xi32>> to vector<2x[4]xi32>
+ // CHECK: return %[[RESULT]] : vector<2x[4]xi32>
+ %result = arith.remsi %lhs, %rhs : vector<2x[4]xi32>
+ return %result : vector<2x[4]xi32>
+}
+
+// -----
+
// Checking conversion of index types to integers using i1, assuming no target
// system would have a 1-bit address space. Otherwise, we would have had to
// make this test dependent on the pointer size on the target system.
``````````
</details>
https://github.com/llvm/llvm-project/pull/233161
More information about the Mlir-commits
mailing list