Skip to content

Commit 9349a10

Browse files
jfurtekjfurtek
authored
Fix side effects for LLVM integer operations (udiv, sdiv) incorrectly marked as Pure (#166648)
This MR modifies side effect traits of some integer arithmetic operations in the LLVM dialect. Prior to this MR, the LLVM dialect `sdiv` and `udiv` operations were marked as `Pure` through `tblgen` inheritance of the `LLVM_ArithmeticOpBase` class. The `Pure` trait allowed incorrect hoisting of `sdiv`/`udiv` operations by the `loop-independent-code-motion` pass. This MR modifies the `sdiv` and `udiv` LLVM operations to have traits and code motion behavior identical to their counterparts in the `arith` dialect, which were established by the commit/review below. ed39825 https://reviews.llvm.org/D137814
1 parent a770d2b commit 9349a10

File tree

4 files changed

+192
-15
lines changed

4 files changed

+192
-15
lines changed

‎mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class LLVM_TerminatorOp<string mnemonic, list<Trait> traits = []> :
3939
class LLVM_ArithmeticOpBase<Type type, string mnemonic,
4040
string instName, list<Trait> traits = []> :
4141
LLVM_Op<mnemonic,
42-
!listconcat([Pure, SameOperandsAndResultType], traits)>,
42+
!listconcat([SameOperandsAndResultType, NoMemoryEffect], traits)>,
4343
LLVM_Builder<"$res = builder.Create" # instName # "($lhs, $rhs);"> {
4444
dag commonArgs = (ins LLVM_ScalarOrVectorOf<type>:$lhs,
4545
LLVM_ScalarOrVectorOf<type>:$rhs);
@@ -116,7 +116,8 @@ class LLVM_IntArithmeticOpWithDisjointFlag<string mnemonic, string instName,
116116
class LLVM_FloatArithmeticOp<string mnemonic, string instName,
117117
list<Trait> traits = []> :
118118
LLVM_ArithmeticOpBase<LLVM_AnyFloat, mnemonic, instName,
119-
!listconcat([DeclareOpInterfaceMethods<FastmathFlagsInterface>], traits)> {
119+
!listconcat([DeclareOpInterfaceMethods<FastmathFlagsInterface>, Pure],
120+
traits)> {
120121
dag fmfArg = (
121122
ins DefaultValuedAttr<LLVM_FastmathFlagsAttr, "{}">:$fastmathFlags);
122123
let arguments = !con(commonArgs, fmfArg);
@@ -149,24 +150,26 @@ class LLVM_UnaryFloatArithmeticOp<Type type, string mnemonic,
149150

150151
// Integer binary operations.
151152
def LLVM_AddOp : LLVM_IntArithmeticOpWithOverflowFlag<"add", "Add",
152-
[Commutative]>;
153-
def LLVM_SubOp : LLVM_IntArithmeticOpWithOverflowFlag<"sub", "Sub", []>;
153+
[Commutative, Pure]>;
154+
def LLVM_SubOp : LLVM_IntArithmeticOpWithOverflowFlag<"sub", "Sub", [Pure]>;
154155
def LLVM_MulOp : LLVM_IntArithmeticOpWithOverflowFlag<"mul", "Mul",
155-
[Commutative]>;
156-
def LLVM_UDivOp : LLVM_IntArithmeticOpWithExactFlag<"udiv", "UDiv">;
157-
def LLVM_SDivOp : LLVM_IntArithmeticOpWithExactFlag<"sdiv", "SDiv">;
158-
def LLVM_URemOp : LLVM_IntArithmeticOp<"urem", "URem">;
159-
def LLVM_SRemOp : LLVM_IntArithmeticOp<"srem", "SRem">;
160-
def LLVM_AndOp : LLVM_IntArithmeticOp<"and", "And">;
161-
def LLVM_OrOp : LLVM_IntArithmeticOpWithDisjointFlag<"or", "Or"> {
156+
[Commutative, Pure]>;
157+
def LLVM_UDivOp : LLVM_IntArithmeticOpWithExactFlag<"udiv", "UDiv",
158+
[DeclareOpInterfaceMethods<ConditionallySpeculatable>]>;
159+
def LLVM_SDivOp : LLVM_IntArithmeticOpWithExactFlag<"sdiv", "SDiv",
160+
[DeclareOpInterfaceMethods<ConditionallySpeculatable>]>;
161+
def LLVM_URemOp : LLVM_IntArithmeticOp<"urem", "URem", [Pure]>;
162+
def LLVM_SRemOp : LLVM_IntArithmeticOp<"srem", "SRem", [Pure]>;
163+
def LLVM_AndOp : LLVM_IntArithmeticOp<"and", "And", [Pure]>;
164+
def LLVM_OrOp : LLVM_IntArithmeticOpWithDisjointFlag<"or", "Or", [Pure]> {
162165
let hasFolder = 1;
163166
}
164-
def LLVM_XOrOp : LLVM_IntArithmeticOp<"xor", "Xor">;
165-
def LLVM_ShlOp : LLVM_IntArithmeticOpWithOverflowFlag<"shl", "Shl", []> {
167+
def LLVM_XOrOp : LLVM_IntArithmeticOp<"xor", "Xor", [Pure]>;
168+
def LLVM_ShlOp : LLVM_IntArithmeticOpWithOverflowFlag<"shl", "Shl", [Pure]> {
166169
let hasFolder = 1;
167170
}
168-
def LLVM_LShrOp : LLVM_IntArithmeticOpWithExactFlag<"lshr", "LShr">;
169-
def LLVM_AShrOp : LLVM_IntArithmeticOpWithExactFlag<"ashr", "AShr">;
171+
def LLVM_LShrOp : LLVM_IntArithmeticOpWithExactFlag<"lshr", "LShr", [Pure]>;
172+
def LLVM_AShrOp : LLVM_IntArithmeticOpWithExactFlag<"ashr", "AShr", [Pure]>;
170173

171174
// Base class for compare operations. A compare operation takes two operands
172175
// of the same type and returns a boolean result. If the operands are

‎mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4223,6 +4223,34 @@ LogicalResult InlineAsmOp::verify() {
42234223
return success();
42244224
}
42254225

4226+
//===----------------------------------------------------------------------===//
4227+
// UDivOp
4228+
//===----------------------------------------------------------------------===//
4229+
Speculation::Speculatability UDivOp::getSpeculatability() {
4230+
// X / 0 => UB
4231+
Value divisor = getRhs();
4232+
if (matchPattern(divisor, m_IntRangeWithoutZeroU()))
4233+
return Speculation::Speculatable;
4234+
4235+
return Speculation::NotSpeculatable;
4236+
}
4237+
4238+
//===----------------------------------------------------------------------===//
4239+
// SDivOp
4240+
//===----------------------------------------------------------------------===//
4241+
Speculation::Speculatability SDivOp::getSpeculatability() {
4242+
// This function conservatively assumes that all signed division by -1 are
4243+
// not speculatable.
4244+
// X / 0 => UB
4245+
// INT_MIN / -1 => UB
4246+
Value divisor = getRhs();
4247+
if (matchPattern(divisor, m_IntRangeWithoutZeroS()) &&
4248+
matchPattern(divisor, m_IntRangeWithoutNegOneS()))
4249+
return Speculation::Speculatable;
4250+
4251+
return Speculation::NotSpeculatable;
4252+
}
4253+
42264254
//===----------------------------------------------------------------------===//
42274255
// LLVMDialect initialization, type parsing, and registration.
42284256
//===----------------------------------------------------------------------===//

‎mlir/test/Transforms/loop-invariant-code-motion.mlir‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,18 @@ func.func @no_speculate_divui(
880880
return
881881
}
882882

883+
func.func @no_speculate_udiv(
884+
// CHECK-LABEL: @no_speculate_udiv(
885+
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
886+
scf.for %i = %lb to %ub step %step {
887+
// CHECK: scf.for
888+
// CHECK: llvm.udiv
889+
%val = llvm.udiv %num, %denom : i32
890+
}
891+
892+
return
893+
}
894+
883895
func.func @no_speculate_divsi(
884896
// CHECK-LABEL: @no_speculate_divsi(
885897
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
@@ -892,6 +904,18 @@ func.func @no_speculate_divsi(
892904
return
893905
}
894906

907+
func.func @no_speculate_sdiv(
908+
// CHECK-LABEL: @no_speculate_sdiv(
909+
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
910+
scf.for %i = %lb to %ub step %step {
911+
// CHECK: scf.for
912+
// CHECK: llvm.sdiv
913+
%val = llvm.sdiv %num, %denom : i32
914+
}
915+
916+
return
917+
}
918+
895919
func.func @no_speculate_ceildivui(
896920
// CHECK-LABEL: @no_speculate_ceildivui(
897921
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
@@ -928,6 +952,18 @@ func.func @no_speculate_divui_const(%num: i32, %lb: index, %ub: index, %step: in
928952
return
929953
}
930954

955+
func.func @no_speculate_udiv_const(%num: i32, %lb: index, %ub: index, %step: index) {
956+
// CHECK-LABEL: @no_speculate_udiv_const(
957+
%c0 = arith.constant 0 : i32
958+
scf.for %i = %lb to %ub step %step {
959+
// CHECK: scf.for
960+
// CHECK: llvm.udiv
961+
%val = llvm.udiv %num, %c0 : i32
962+
}
963+
964+
return
965+
}
966+
931967
func.func @speculate_divui_const(
932968
// CHECK-LABEL: @speculate_divui_const(
933969
%num: i32, %lb: index, %ub: index, %step: index) {
@@ -941,6 +977,19 @@ func.func @speculate_divui_const(
941977
return
942978
}
943979

980+
func.func @speculate_udiv_const(
981+
// CHECK-LABEL: @speculate_udiv_const(
982+
%num: i32, %lb: index, %ub: index, %step: index) {
983+
%c5 = llvm.mlir.constant(5 : i32) : i32
984+
// CHECK: llvm.udiv
985+
// CHECK: scf.for
986+
scf.for %i = %lb to %ub step %step {
987+
%val = llvm.udiv %num, %c5 : i32
988+
}
989+
990+
return
991+
}
992+
944993
func.func @no_speculate_ceildivui_const(%num: i32, %lb: index, %ub: index, %step: index) {
945994
// CHECK-LABEL: @no_speculate_ceildivui_const(
946995
%c0 = arith.constant 0 : i32
@@ -979,6 +1028,19 @@ func.func @no_speculate_divsi_const0(
9791028
return
9801029
}
9811030

1031+
func.func @no_speculate_sdiv_const0(
1032+
// CHECK-LABEL: @no_speculate_sdiv_const0(
1033+
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
1034+
%c0 = arith.constant 0 : i32
1035+
scf.for %i = %lb to %ub step %step {
1036+
// CHECK: scf.for
1037+
// CHECK: llvm.sdiv
1038+
%val = llvm.sdiv %num, %c0 : i32
1039+
}
1040+
1041+
return
1042+
}
1043+
9821044
func.func @no_speculate_divsi_const_minus1(
9831045
// CHECK-LABEL: @no_speculate_divsi_const_minus1(
9841046
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
@@ -992,6 +1054,19 @@ func.func @no_speculate_divsi_const_minus1(
9921054
return
9931055
}
9941056

1057+
func.func @no_speculate_sdiv_const_minus1(
1058+
// CHECK-LABEL: @no_speculate_sdiv_const_minus1(
1059+
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
1060+
%cm1 = arith.constant -1 : i32
1061+
scf.for %i = %lb to %ub step %step {
1062+
// CHECK: scf.for
1063+
// CHECK: llvm.sdiv
1064+
%val = llvm.sdiv %num, %cm1 : i32
1065+
}
1066+
1067+
return
1068+
}
1069+
9951070
func.func @speculate_divsi_const(
9961071
// CHECK-LABEL: @speculate_divsi_const(
9971072
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
@@ -1005,6 +1080,19 @@ func.func @speculate_divsi_const(
10051080
return
10061081
}
10071082

1083+
func.func @speculate_sdiv_const(
1084+
// CHECK-LABEL: @speculate_sdiv_const(
1085+
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
1086+
%c5 = arith.constant 5 : i32
1087+
scf.for %i = %lb to %ub step %step {
1088+
// CHECK: llvm.sdiv
1089+
// CHECK: scf.for
1090+
%val = llvm.sdiv %num, %c5 : i32
1091+
}
1092+
1093+
return
1094+
}
1095+
10081096
func.func @no_speculate_ceildivsi_const0(
10091097
// CHECK-LABEL: @no_speculate_ceildivsi_const0(
10101098
%num: i32, %denom: i32, %lb: index, %ub: index, %step: index) {
@@ -1057,6 +1145,19 @@ func.func @no_speculate_divui_range(
10571145
return
10581146
}
10591147

1148+
func.func @no_speculate_udiv_range(
1149+
// CHECK-LABEL: @no_speculate_udiv_range(
1150+
%num: i8, %lb: index, %ub: index, %step: index) {
1151+
%denom = test.with_bounds {smax = 127 : i8, smin = -128 : i8, umax = 255 : i8, umin = 0 : i8} : i8
1152+
scf.for %i = %lb to %ub step %step {
1153+
// CHECK: scf.for
1154+
// CHECK: llvm.udiv
1155+
%val = llvm.udiv %num, %denom : i8
1156+
}
1157+
1158+
return
1159+
}
1160+
10601161
func.func @no_speculate_divsi_range(
10611162
// CHECK-LABEL: @no_speculate_divsi_range(
10621163
%num: i8, %lb: index, %ub: index, %step: index) {
@@ -1072,6 +1173,21 @@ func.func @no_speculate_divsi_range(
10721173
return
10731174
}
10741175

1176+
func.func @no_speculate_sdiv_range(
1177+
// CHECK-LABEL: @no_speculate_sdiv_range(
1178+
%num: i8, %lb: index, %ub: index, %step: index) {
1179+
%denom0 = test.with_bounds {smax = -1: i8, smin = -128 : i8, umax = 255 : i8, umin = 0 : i8} : i8
1180+
%denom1 = test.with_bounds {smax = 127 : i8, smin = 0 : i8, umax = 255 : i8, umin = 0 : i8} : i8
1181+
scf.for %i = %lb to %ub step %step {
1182+
// CHECK: scf.for
1183+
// CHECK-COUNT-2: llvm.sdiv
1184+
%val0 = llvm.sdiv %num, %denom0 : i8
1185+
%val1 = llvm.sdiv %num, %denom1 : i8
1186+
}
1187+
1188+
return
1189+
}
1190+
10751191
func.func @no_speculate_ceildivui_range(
10761192
// CHECK-LABEL: @no_speculate_ceildivui_range(
10771193
%num: i8, %lb: index, %ub: index, %step: index) {
@@ -1113,6 +1229,19 @@ func.func @speculate_divui_range(
11131229
return
11141230
}
11151231

1232+
func.func @speculate_udiv_range(
1233+
// CHECK-LABEL: @speculate_udiv_range(
1234+
%num: i8, %lb: index, %ub: index, %step: index) {
1235+
%denom = test.with_bounds {smax = 127 : i8, smin = -128 : i8, umax = 255 : i8, umin = 1 : i8} : i8
1236+
scf.for %i = %lb to %ub step %step {
1237+
// CHECK: llvm.udiv
1238+
// CHECK: scf.for
1239+
%val = llvm.udiv %num, %denom : i8
1240+
}
1241+
1242+
return
1243+
}
1244+
11161245
func.func @speculate_divsi_range(
11171246
// CHECK-LABEL: @speculate_divsi_range(
11181247
%num: i8, %lb: index, %ub: index, %step: index) {
@@ -1129,6 +1258,22 @@ func.func @speculate_divsi_range(
11291258
return
11301259
}
11311260

1261+
func.func @speculate_sdiv_range(
1262+
// CHECK-LABEL: @speculate_sdiv_range(
1263+
%num: i8, %lb: index, %ub: index, %step: index) {
1264+
%denom0 = test.with_bounds {smax = 127 : i8, smin = 1 : i8, umax = 255 : i8, umin = 0 : i8} : i8
1265+
%denom1 = test.with_bounds {smax = -2 : i8, smin = -128 : i8, umax = 255 : i8, umin = 0 : i8} : i8
1266+
scf.for %i = %lb to %ub step %step {
1267+
// CHECK-COUNT-2: llvm.sdiv
1268+
// CHECK: scf.for
1269+
%val0 = llvm.sdiv %num, %denom0 : i8
1270+
%val1 = llvm.sdiv %num, %denom1 : i8
1271+
1272+
}
1273+
1274+
return
1275+
}
1276+
11321277
func.func @speculate_ceildivui_range(
11331278
// CHECK-LABEL: @speculate_ceildivui_range(
11341279
%num: i8, %lb: index, %ub: index, %step: index) {

‎mlir/unittests/Dialect/LLVMIR/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ add_mlir_unittest(MLIRLLVMIRTests
44
mlir_target_link_libraries(MLIRLLVMIRTests
55
PRIVATE
66
MLIRLLVMDialect
7+
MLIRInferIntRangeInterface
78
)

0 commit comments

Comments
 (0)