Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit bea559b

Browse files
committed
[DAG] vector div/rem with any zero element in divisor is undef
This is the backend counterpart to: https://reviews.llvm.org/rL297390 https://reviews.llvm.org/rL297409 and follow-up to: https://reviews.llvm.org/rL297384 It surprised me that we need to duplicate the check in FoldConstantArithmetic and FoldConstantVectorArithmetic, but one or the other doesn't catch all of the test cases. There is an existing code comment about merging those someday. Differential Revision: https://reviews.llvm.org/D30826 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@297762 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent cd2a5b6 commit bea559b

File tree

5 files changed

+35
-33
lines changed

5 files changed

+35
-33
lines changed

include/llvm/CodeGen/SelectionDAG.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,9 @@ class SelectionDAG {
740740
return getNode(ISD::CALLSEQ_END, DL, NodeTys, Ops);
741741
}
742742

743+
/// Return true if the result of this operation is always undefined.
744+
bool isUndef(unsigned Opcode, ArrayRef<SDValue> Ops);
745+
743746
/// Return an UNDEF node. UNDEF does not have a useful SDLoc.
744747
SDValue getUNDEF(EVT VT) {
745748
return getNode(ISD::UNDEF, SDLoc(), VT);

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2524,15 +2524,7 @@ static SDValue simplifyDivRem(SDNode *N, SelectionDAG &DAG) {
25242524
EVT VT = N->getValueType(0);
25252525
SDLoc DL(N);
25262526

2527-
// X / undef -> undef
2528-
// X % undef -> undef
2529-
if (N1.isUndef())
2530-
return N1;
2531-
2532-
// X / 0 --> undef
2533-
// X % 0 --> undef
2534-
// We don't need to preserve faults!
2535-
if (isNullConstantOrNullSplatConstant(N1))
2527+
if (DAG.isUndef(N->getOpcode(), {N0, N1}))
25362528
return DAG.getUNDEF(VT);
25372529

25382530
// undef / X -> 0

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3695,12 +3695,6 @@ SDValue SelectionDAG::FoldConstantArithmetic(unsigned Opcode, const SDLoc &DL,
36953695
if (Cst1->isOpaque() || Cst2->isOpaque())
36963696
return SDValue();
36973697

3698-
// Division/remainder with a zero divisor is undefined behavior.
3699-
if ((Opcode == ISD::SDIV || Opcode == ISD::UDIV ||
3700-
Opcode == ISD::SREM || Opcode == ISD::UREM) &&
3701-
Cst2->isNullValue())
3702-
return getUNDEF(VT);
3703-
37043698
std::pair<APInt, bool> Folded = FoldValue(Opcode, Cst1->getAPIntValue(),
37053699
Cst2->getAPIntValue());
37063700
if (!Folded.second)
@@ -3728,6 +3722,30 @@ SDValue SelectionDAG::FoldSymbolOffset(unsigned Opcode, EVT VT,
37283722
GA->getOffset() + uint64_t(Offset));
37293723
}
37303724

3725+
bool SelectionDAG::isUndef(unsigned Opcode, ArrayRef<SDValue> Ops) {
3726+
switch (Opcode) {
3727+
case ISD::SDIV:
3728+
case ISD::UDIV:
3729+
case ISD::SREM:
3730+
case ISD::UREM: {
3731+
// If a divisor is zero/undef or any element of a divisor vector is
3732+
// zero/undef, the whole op is undef.
3733+
assert(Ops.size() == 2 && "Div/rem should have 2 operands");
3734+
SDValue Divisor = Ops[1];
3735+
if (Divisor.isUndef() || isNullConstant(Divisor))
3736+
return true;
3737+
3738+
return ISD::isBuildVectorOfConstantSDNodes(Divisor.getNode()) &&
3739+
any_of(Divisor->op_values(),
3740+
[](SDValue V) { return V.isUndef() || isNullConstant(V); });
3741+
// TODO: Handle signed overflow.
3742+
}
3743+
// TODO: Handle oversized shifts.
3744+
default:
3745+
return false;
3746+
}
3747+
}
3748+
37313749
SDValue SelectionDAG::FoldConstantArithmetic(unsigned Opcode, const SDLoc &DL,
37323750
EVT VT, SDNode *Cst1,
37333751
SDNode *Cst2) {
@@ -3737,6 +3755,9 @@ SDValue SelectionDAG::FoldConstantArithmetic(unsigned Opcode, const SDLoc &DL,
37373755
if (Opcode >= ISD::BUILTIN_OP_END)
37383756
return SDValue();
37393757

3758+
if (isUndef(Opcode, {SDValue(Cst1, 0), SDValue(Cst2, 0)}))
3759+
return getUNDEF(VT);
3760+
37403761
// Handle the case of two scalars.
37413762
if (const ConstantSDNode *Scalar1 = dyn_cast<ConstantSDNode>(Cst1)) {
37423763
if (const ConstantSDNode *Scalar2 = dyn_cast<ConstantSDNode>(Cst2)) {
@@ -3804,6 +3825,9 @@ SDValue SelectionDAG::FoldConstantVectorArithmetic(unsigned Opcode,
38043825
if (Opcode >= ISD::BUILTIN_OP_END)
38053826
return SDValue();
38063827

3828+
if (isUndef(Opcode, Ops))
3829+
return getUNDEF(VT);
3830+
38073831
// We can only fold vectors - maybe merge with FoldConstantArithmetic someday?
38083832
if (!VT.isVector())
38093833
return SDValue();

test/CodeGen/X86/div-rem-simplify.ll

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ define <4 x i32> @sel_sdiv0_vec(i1 %cond) {
152152
define <4 x i32> @sdiv0elt_vec(<4 x i32> %x) {
153153
; CHECK-LABEL: sdiv0elt_vec:
154154
; CHECK: # BB#0:
155-
; CHECK-NEXT: movaps {{.*#+}} xmm0 = <u,12,u,4294967292>
156155
; CHECK-NEXT: retq
157156
%zero = and <4 x i32> %x, <i32 0, i32 0, i32 0, i32 0>
158157
%some_ones = or <4 x i32> %zero, <i32 0, i32 -1, i32 0, i32 3>
@@ -163,7 +162,6 @@ define <4 x i32> @sdiv0elt_vec(<4 x i32> %x) {
163162
define <4 x i32> @udiv0elt_vec(<4 x i32> %x) {
164163
; CHECK-LABEL: udiv0elt_vec:
165164
; CHECK: # BB#0:
166-
; CHECK-NEXT: movaps {{.*#+}} xmm0 = <u,4,3,u>
167165
; CHECK-NEXT: retq
168166
%div = udiv <4 x i32> <i32 11, i32 12, i32 13, i32 14>, <i32 0, i32 3, i32 4, i32 0>
169167
ret <4 x i32> %div
@@ -172,7 +170,6 @@ define <4 x i32> @udiv0elt_vec(<4 x i32> %x) {
172170
define <4 x i32> @urem0elt_vec(<4 x i32> %x) {
173171
; CHECK-LABEL: urem0elt_vec:
174172
; CHECK: # BB#0:
175-
; CHECK-NEXT: movaps {{.*#+}} xmm0 = <u,u,u,2>
176173
; CHECK-NEXT: retq
177174
%zero = and <4 x i32> %x, <i32 0, i32 0, i32 0, i32 0>
178175
%some_ones = or <4 x i32> %zero, <i32 0, i32 0, i32 0, i32 3>
@@ -183,8 +180,6 @@ define <4 x i32> @urem0elt_vec(<4 x i32> %x) {
183180
define <4 x i32> @srem0elt_vec(<4 x i32> %x) {
184181
; CHECK-LABEL: srem0elt_vec:
185182
; CHECK: # BB#0:
186-
; CHECK-NEXT: movl $-2, %eax
187-
; CHECK-NEXT: movd %eax, %xmm0
188183
; CHECK-NEXT: retq
189184
%rem = srem <4 x i32> <i32 -11, i32 -12, i32 -13, i32 -14>, <i32 -3, i32 -3, i32 0, i32 2>
190185
ret <4 x i32> %rem

test/CodeGen/X86/vec_sdiv_to_shift.ll

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -184,27 +184,15 @@ entry:
184184
ret <16 x i16> %a0
185185
}
186186

187-
; TODO: The div-by-0 lanes are folded away, so we use scalar ops. Would it be better to keep this in the vector unit?
187+
; Div-by-0 in any lane is UB.
188188

189189
define <4 x i32> @sdiv_non_splat(<4 x i32> %x) {
190190
; SSE-LABEL: sdiv_non_splat:
191191
; SSE: # BB#0:
192-
; SSE-NEXT: movd %xmm0, %eax
193-
; SSE-NEXT: movl %eax, %ecx
194-
; SSE-NEXT: shrl $31, %ecx
195-
; SSE-NEXT: addl %eax, %ecx
196-
; SSE-NEXT: sarl %ecx
197-
; SSE-NEXT: movd %ecx, %xmm0
198192
; SSE-NEXT: retq
199193
;
200194
; AVX-LABEL: sdiv_non_splat:
201195
; AVX: # BB#0:
202-
; AVX-NEXT: vmovd %xmm0, %eax
203-
; AVX-NEXT: movl %eax, %ecx
204-
; AVX-NEXT: shrl $31, %ecx
205-
; AVX-NEXT: addl %eax, %ecx
206-
; AVX-NEXT: sarl %ecx
207-
; AVX-NEXT: vmovd %ecx, %xmm0
208196
; AVX-NEXT: retq
209197
%y = sdiv <4 x i32> %x, <i32 2, i32 0, i32 0, i32 0>
210198
ret <4 x i32> %y

0 commit comments

Comments
 (0)