<div dir="ltr">Thanks, Mikael. <br><br>Yes, we can't always propagate the existing undef operand in the DAG; the sizes may not match. Seems like a small fix. Taking a look now.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 13, 2018 at 2:56 AM, Mikael Holmén <span dir="ltr"><<a href="mailto:mikael.holmen@ericsson.com" target="_blank">mikael.holmen@ericsson.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Sanjay,<br>
<br>
I ran into problems with this patch.<br>
<br>
The problem is that an i32:<br>
<br>
%_tmp1905.i = shl i32 7, undef<br>
<br>
is changed into an i8:<br>
<br>
Creating new node: t3: i8 = undef<br>
<br>
so the folding of<br>
<br>
op(arg1, undef) -> undef<br>
<br>
actually changed the type of the result of the shl from i32 to i8.<br>
<br>
The result is then used in a setcc<br>
<br>
Creating new node: t5: i1 = setcc undef:i32, undef:i8, seteq:ch<br>
<br>
and then an assert triggers about the operands in setcc having different sizes.<br>
<br>
llc: ../lib/CodeGen/SelectionDAG/DA<wbr>GCombiner.cpp:7764: bool isTruncateOf(llvm::SelectionDA<wbr>G &, llvm::SDValue, llvm::SDValue &, llvm::KnownBits &): Assertion `Op0.getValueType() == Op1.getValueType()' failed.<br>
<br>
Reproduce with:<br>
llc shl_undef.ll -o /dev/null -mtriple=x86_64-linux<br>
<br>
Regards,<br>
Mikael<div class="HOEnZb"><div class="h5"><br>
<br>
On 02/12/2018 10:37 PM, Sanjay Patel via llvm-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: spatel<br>
Date: Mon Feb 12 13:37:27 2018<br>
New Revision: 324941<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=324941&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=324941&view=rev</a><br>
Log:<br>
[DAG] make binops with undef operands consistent with IR<br>
<br>
This started by noticing that scalar and vector types were producing different results with div ops in PR36305:<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=36305" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug<wbr>.cgi?id=36305</a><br>
<br>
...but the problem is bigger. I couldn't keep it straight without a table, so I'm attaching that as a PDF to<br>
the review. The x86 tests in undef-ops.ll correspond to that table.<br>
<br>
Green means that instsimplify and the DAG agree on the result for all types.<br>
Red means the DAG was returning undef when IR was not.<br>
Yellow means the DAG was returning a non-undef result when IR returned undef.<br>
<br>
This patch assumes that we're currently doing the right thing in IR.<br>
<br>
Note: I couldn't find any problems with lowering vector constants as the code comments were warning,<br>
but those comments were written long ago in rL36413 .<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D43141" rel="noreferrer" target="_blank">https://reviews.llvm.org/D4314<wbr>1</a><br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/Select<wbr>ionDAG/SelectionDAG.cpp<br>
llvm/trunk/test/CodeGen/Hexag<wbr>on/tail-dup-subreg-map.ll<br>
llvm/trunk/test/CodeGen/X86/p<wbr>r13577.ll<br>
llvm/trunk/test/CodeGen/X86/p<wbr>r33960.ll<br>
llvm/trunk/test/CodeGen/X86/u<wbr>ndef-ops.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=324941&r1=324940&r2=324941&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/SelectionDAG.cpp?<wbr>rev=324941&r1=324940&r2=<wbr>324941&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAG.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/Selecti<wbr>onDAG/SelectionDAG.cpp Mon Feb 12 13:37:27 2018<br>
@@ -4667,19 +4667,15 @@ SDValue SelectionDAG::getNode(unsigned O<br>
case ISD::FSUB:<br>
case ISD::FDIV:<br>
case ISD::FREM:<br>
- case ISD::SRA:<br>
return N1; // fold op(undef, arg2) -> undef<br>
case ISD::UDIV:<br>
case ISD::SDIV:<br>
case ISD::UREM:<br>
case ISD::SREM:<br>
+ case ISD::SRA:<br>
case ISD::SRL:<br>
case ISD::SHL:<br>
- if (!VT.isVector())<br>
- return getConstant(0, DL, VT); // fold op(undef, arg2) -> 0<br>
- // For vectors, we can't easily build an all zero vector, just return<br>
- // the LHS.<br>
- return N2;<br>
+ return getConstant(0, DL, VT); // fold op(undef, arg2) -> 0<br>
}<br>
}<br>
}<br>
@@ -4701,6 +4697,9 @@ SDValue SelectionDAG::getNode(unsigned O<br>
case ISD::SDIV:<br>
case ISD::UREM:<br>
case ISD::SREM:<br>
+ case ISD::SRA:<br>
+ case ISD::SRL:<br>
+ case ISD::SHL:<br>
return N2; // fold op(arg1, undef) -> undef<br>
case ISD::FADD:<br>
case ISD::FSUB:<br>
@@ -4712,21 +4711,9 @@ SDValue SelectionDAG::getNode(unsigned O<br>
break;<br>
case ISD::MUL:<br>
case ISD::AND:<br>
- case ISD::SRL:<br>
- case ISD::SHL:<br>
- if (!VT.isVector())<br>
- return getConstant(0, DL, VT); // fold op(arg1, undef) -> 0<br>
- // For vectors, we can't easily build an all zero vector, just return<br>
- // the LHS.<br>
- return N1;<br>
+ return getConstant(0, DL, VT); // fold op(arg1, undef) -> 0<br>
case ISD::OR:<br>
- if (!VT.isVector())<br>
- return getConstant(APInt::getAllOnesV<wbr>alue(VT.getSizeInBits()), DL, VT);<br>
- // For vectors, we can't easily build an all one vector, just return<br>
- // the LHS.<br>
- return N1;<br>
- case ISD::SRA:<br>
- return N1;<br>
+ return getAllOnesConstant(DL, VT);<br>
}<br>
}<br>
<br>
Modified: llvm/trunk/test/CodeGen/Hexago<wbr>n/tail-dup-subreg-map.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/tail-dup-subreg-map.ll?rev=324941&r1=324940&r2=324941&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/CodeGen/<wbr>Hexagon/tail-dup-subreg-map.<wbr>ll?rev=324941&r1=324940&r2=<wbr>324941&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/Hexago<wbr>n/tail-dup-subreg-map.ll (original)<br>
+++ llvm/trunk/test/CodeGen/Hexago<wbr>n/tail-dup-subreg-map.ll Mon Feb 12 13:37:27 2018<br>
@@ -5,7 +5,7 @@<br>
; subregisters were dropped by the tail duplicator, resulting in invalid<br>
; COPY instructions being generated.<br>
-; CHECK: = extractu(r{{[0-9]+}},#15,#17)<br>
+; CHECK: = asl(r{{[0-9]+}}:{{[0-9]+}},#15<wbr>)<br>
target triple = "hexagon"<br>
@@ -36,20 +36,20 @@ if.then5.i:<br>
br label %if.end.i<br>
if.else.i: ; preds = %if.then.i<br>
- %shl12.i = shl i64 %0, undef<br>
+ %shl12.i = shl i64 %0, 7<br>
br label %if.end.i<br>
if.end.i: ; preds = %if.else.i, %if.then5.i<br>
%aSig0.0 = phi i64 [ undef, %if.then5.i ], [ %shl12.i, %if.else.i ]<br>
%storemerge43.i = phi i64 [ %shl.i21, %if.then5.i ], [ 0, %if.else.i ]<br>
- %sub15.i = sub nsw i32 -63, undef<br>
+ %sub15.i = sub nsw i32 -63, 8<br>
br label %if.end13<br>
if.else16.i: ; preds = %if.then7<br>
br label %if.end13<br>
if.else: ; preds = %entry<br>
- %or12 = or i64 undef, 281474976710656<br>
+ %or12 = or i64 9, 281474976710656<br>
br label %if.end13<br>
if.end13: ; preds = %if.else, %if.else16.i, %if.end.i<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/pr<wbr>13577.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr13577.ll?rev=324941&r1=324940&r2=324941&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/CodeGen/<wbr>X86/pr13577.ll?rev=324941&r1=<wbr>324940&r2=324941&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/X86/pr<wbr>13577.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/pr<wbr>13577.ll Mon Feb 12 13:37:27 2018<br>
@@ -31,7 +31,6 @@ define float @pr26070() {<br>
; CHECK: ## %bb.0:<br>
; CHECK-NEXT: movss {{.*#+}} xmm0 = mem[0],zero,zero,zero<br>
; CHECK-NEXT: shufps {{.*#+}} xmm0 = xmm0[0,0,0,0]<br>
-; CHECK-NEXT: orps {{.*}}(%rip), %xmm0<br>
; CHECK-NEXT: retq<br>
%c = call float @copysignf(float 1.0, float undef) readnone<br>
ret float %c<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/pr<wbr>33960.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr33960.ll?rev=324941&r1=324940&r2=324941&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/CodeGen/<wbr>X86/pr33960.ll?rev=324941&r1=<wbr>324940&r2=324941&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/X86/pr<wbr>33960.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/pr<wbr>33960.ll Mon Feb 12 13:37:27 2018<br>
@@ -7,12 +7,12 @@<br>
define void @PR33960() {<br>
; X86-LABEL: PR33960:<br>
; X86: # %bb.0: # %entry<br>
-; X86-NEXT: movl $0, b<br>
+; X86-NEXT: movl $-1, b<br>
; X86-NEXT: retl<br>
;<br>
; X64-LABEL: PR33960:<br>
; X64: # %bb.0: # %entry<br>
-; X64-NEXT: movl $0, {{.*}}(%rip)<br>
+; X64-NEXT: movl $-1, {{.*}}(%rip)<br>
; X64-NEXT: retq<br>
entry:<br>
%tmp = insertelement <4 x i32> <i32 undef, i32 -7, i32 -3, i32 undef>, i32 -2, i32 3<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/un<wbr>def-ops.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/undef-ops.ll?rev=324941&r1=324940&r2=324941&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/CodeGen/<wbr>X86/undef-ops.ll?rev=324941&<wbr>r1=324940&r2=324941&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/X86/un<wbr>def-ops.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/un<wbr>def-ops.ll Mon Feb 12 13:37:27 2018<br>
@@ -77,6 +77,7 @@ define i32 @mul_undef_rhs(i32 %x) {<br>
define <4 x i32> @mul_undef_rhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: mul_undef_rhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = mul <4 x i32> %x, undef<br>
ret <4 x i32> %r<br>
@@ -94,6 +95,7 @@ define i32 @mul_undef_lhs(i32 %x) {<br>
define <4 x i32> @mul_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: mul_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = mul <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -127,6 +129,7 @@ define i32 @sdiv_undef_lhs(i32 %x) {<br>
define <4 x i32> @sdiv_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: sdiv_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = sdiv <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -160,6 +163,7 @@ define i32 @udiv_undef_lhs(i32 %x) {<br>
define <4 x i32> @udiv_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: udiv_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = udiv <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -193,6 +197,7 @@ define i32 @srem_undef_lhs(i32 %x) {<br>
define <4 x i32> @srem_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: srem_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = srem <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -226,6 +231,7 @@ define i32 @urem_undef_lhs(i32 %x) {<br>
define <4 x i32> @urem_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: urem_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = urem <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -234,7 +240,6 @@ define <4 x i32> @urem_undef_lhs_vec(<4<br>
define i32 @ashr_undef_rhs(i32 %x) {<br>
; CHECK-LABEL: ashr_undef_rhs:<br>
; CHECK: # %bb.0:<br>
-; CHECK-NEXT: movl %edi, %eax<br>
; CHECK-NEXT: retq<br>
%r = ashr i32 %x, undef<br>
ret i32 %r<br>
@@ -251,6 +256,7 @@ define <4 x i32> @ashr_undef_rhs_vec(<4<br>
define i32 @ashr_undef_lhs(i32 %x) {<br>
; CHECK-LABEL: ashr_undef_lhs:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorl %eax, %eax<br>
; CHECK-NEXT: retq<br>
%r = ashr i32 undef, %x<br>
ret i32 %r<br>
@@ -259,6 +265,7 @@ define i32 @ashr_undef_lhs(i32 %x) {<br>
define <4 x i32> @ashr_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: ashr_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = ashr <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -267,7 +274,6 @@ define <4 x i32> @ashr_undef_lhs_vec(<4<br>
define i32 @lshr_undef_rhs(i32 %x) {<br>
; CHECK-LABEL: lshr_undef_rhs:<br>
; CHECK: # %bb.0:<br>
-; CHECK-NEXT: xorl %eax, %eax<br>
; CHECK-NEXT: retq<br>
%r = lshr i32 %x, undef<br>
ret i32 %r<br>
@@ -293,6 +299,7 @@ define i32 @lshr_undef_lhs(i32 %x) {<br>
define <4 x i32> @lshr_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: lshr_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = lshr <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -301,7 +308,6 @@ define <4 x i32> @lshr_undef_lhs_vec(<4<br>
define i32 @shl_undef_rhs(i32 %x) {<br>
; CHECK-LABEL: shl_undef_rhs:<br>
; CHECK: # %bb.0:<br>
-; CHECK-NEXT: xorl %eax, %eax<br>
; CHECK-NEXT: retq<br>
%r = shl i32 %x, undef<br>
ret i32 %r<br>
@@ -327,6 +333,7 @@ define i32 @shl_undef_lhs(i32 %x) {<br>
define <4 x i32> @shl_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: shl_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = shl <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -344,6 +351,7 @@ define i32 @and_undef_rhs(i32 %x) {<br>
define <4 x i32> @and_undef_rhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: and_undef_rhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = and <4 x i32> %x, undef<br>
ret <4 x i32> %r<br>
@@ -361,6 +369,7 @@ define i32 @and_undef_lhs(i32 %x) {<br>
define <4 x i32> @and_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: and_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: xorps %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = and <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
@@ -378,6 +387,7 @@ define i32 @or_undef_rhs(i32 %x) {<br>
define <4 x i32> @or_undef_rhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: or_undef_rhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: pcmpeqd %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = or <4 x i32> %x, undef<br>
ret <4 x i32> %r<br>
@@ -395,6 +405,7 @@ define i32 @or_undef_lhs(i32 %x) {<br>
define <4 x i32> @or_undef_lhs_vec(<4 x i32> %x) {<br>
; CHECK-LABEL: or_undef_lhs_vec:<br>
; CHECK: # %bb.0:<br>
+; CHECK-NEXT: pcmpeqd %xmm0, %xmm0<br>
; CHECK-NEXT: retq<br>
%r = or <4 x i32> undef, %x<br>
ret <4 x i32> %r<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote>
</div></div></blockquote></div><br></div>