[DAG] make binops with undef operands consistent with IR

[spatel]

Summary

This started by noticing that scalar and vector types were producing different results with div ops in PR36305:
https://bugs.llvm.org/show_bug.cgi?id=36305

...but the problem is bigger. I couldn't keep it straight without a table, so I'm attaching that as a PDF to this review. The x86 tests in undef-ops.ll correspond to that table.

Green means that instsimplify and the DAG agree on the result for all types.
Red means the DAG was returning undef when IR was not.
Yellow means the DAG was returning a non-undef result when IR returned undef.

This patch assumes that we're doing the right thing in IR, but now's a good time to confirm that. :)

Note: I couldn't find any problems with creating vector constants as the code comments were warning, but those comments were written long ago in rL36413 .

undef in DAG.pdf40 KBDownload

[efriedma]

Some of the test changes are a little iffy... we might want to adjust the to stop using undef rather than just change the expected result, to try to preserve some of the original intent.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

4713 ↗

(On Diff #133673)

"For vectors, we can't easily build an all zero vector" doesn't mean that it's hard to construct a BUILD_VECTOR; the problem is ensuring we lower it before we run selection. But it might not be a problem anymore because DAGCombine calls the legalizer?

[spatel]

Patch updated:
Edited tests so they are not affected by this patch:
rL324814
rL324816
rL324817

I think the remaining x86 tests show the undef functional change from this patch as intended.
I'm not sure what to do with the Hexagon test - changing some constants as shown didn't restore the current CHECK. @kparzysz - suggestions?

[nlopes]

oh, wow, so many bugs in DAG combiner handling undef.. Nice table btw!

I wonder if the handling of nsw/nuw in SelectionDAG isn't buggy as well, but that's another question..

[kparzysz]

The change in the Hexagon test is fine. The original testcase crashed before the fix, so as long as it compiles successfully, any CHECK will work.

[efriedma]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

if (N1.isUndef()) {
     } else {
       switch (Opcode) {
       case ISD::FP_ROUND_INREG:
       case ISD::SIGN_EXTEND_INREG:
       case ISD::SUB:
       case ISD::FSUB:
       case ISD::FDIV:
       case ISD::FREM:
-      case ISD::SRA:
         return N1;     // fold op(undef, arg2) -> undef
       case ISD::UDIV:
       case ISD::SDIV:
       case ISD::UREM:
       case ISD::SREM:
+      case ISD::SRA:
       case ISD::SRL:
       case ISD::SHL:
-        if (!VT.isVector())
-          return getConstant(0, DL, VT);    // fold op(undef, arg2) -> 0
-        // For vectors, we can't easily build an all zero vector, just return
-        // the LHS.
-        return N2;
       }
     }
   }
 
   // Fold a bunch of operators when the RHS is undef.
   if (N2.isUndef()) {
     switch (Opcode) {
     case ISD::XOR:
       if (N1.isUndef())
         // Handle undef ^ undef -> 0 special case. This is a common
         // idiom (misuse).
         return getConstant(0, DL, VT);
       LLVM_FALLTHROUGH;
     case ISD::ADD:
     case ISD::ADDC:
     case ISD::ADDE:
     case ISD::SUB:
     case ISD::UDIV:
     case ISD::SDIV:
     case ISD::UREM:
     case ISD::SREM:
+    case ISD::SRA:
+    case ISD::SRL:
+    case ISD::SHL:
       return N2;       // fold op(arg1, undef) -> undef
     case ISD::FADD:
     case ISD::FSUB:
     case ISD::FMUL:
     case ISD::FDIV:
     case ISD::FREM:
       if (getTarget().Options.UnsafeFPMath)
         return N2;
       break;
     case ISD::MUL:
     case ISD::AND:
-    case ISD::SRL:
-    case ISD::SHL:
-      if (!VT.isVector())
-        return getConstant(0, DL, VT);  // fold op(arg1, undef) -> 0
-      // For vectors, we can't easily build an all zero vector, just return
-      // the LHS.
-      return N1;
     case ISD::OR:
-      if (!VT.isVector())
-        return getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), DL, VT);
-      // For vectors, we can't easily build an all one vector, just return
-      // the LHS.
-      return N1;
-    case ISD::SRA:
-      return N1;
+      return getAllOnesConstant(DL, VT);
     }
   }
 
   // Memoize this node if possible.
   SDNode *N;
   SDVTList VTs = getVTList(VT);
   SDValue Ops[] = {N1, N2};
   if (VT != MVT::Glue) {

llvm/trunk/test/CodeGen/Hexagon/tail-dup-subreg-map.ll

 ; RUN: llc -march=hexagon < %s | FileCheck %s
 ; REQUIRES: asserts
 
 ; When tail-duplicating a block with PHI nodes that use subregisters, the
 ; subregisters were dropped by the tail duplicator, resulting in invalid
 ; COPY instructions being generated.
 
-; CHECK: = extractu(r{{[0-9]+}},#15,#17)
+; CHECK: = asl(r{{[0-9]+}}:{{[0-9]+}},#15)
 
 target triple = "hexagon"
 
 %struct.0 = type { i64, i16 }
 %struct.1 = type { i64, i64 }
 
 declare hidden fastcc void @foo(%struct.0* noalias nocapture, i8 signext, i8 zeroext, i32, i64, i64) unnamed_addr #0
 

 if.then.i:                                        ; preds = %if.then7
   br i1 undef, label %if.then5.i, label %if.else.i
 
 if.then5.i:                                       ; preds = %if.then.i
   %shl.i21 = shl i64 %0, 0
   br label %if.end.i
 
 if.else.i:                                        ; preds = %if.then.i
-  %shl12.i = shl i64 %0, undef
+  %shl12.i = shl i64 %0, 7
   br label %if.end.i
 
 if.end.i:                                         ; preds = %if.else.i, %if.then5.i
   %aSig0.0 = phi i64 [ undef, %if.then5.i ], [ %shl12.i, %if.else.i ]
   %storemerge43.i = phi i64 [ %shl.i21, %if.then5.i ], [ 0, %if.else.i ]
-  %sub15.i = sub nsw i32 -63, undef
+  %sub15.i = sub nsw i32 -63, 8
   br label %if.end13
 
 if.else16.i:                                      ; preds = %if.then7
   br label %if.end13
 
 if.else:                                          ; preds = %entry
-  %or12 = or i64 undef, 281474976710656
+  %or12 = or i64 9, 281474976710656
   br label %if.end13
 
 if.end13:                                         ; preds = %if.else, %if.else16.i, %if.end.i
   %aSig1.1 = phi i64 [ %0, %if.else ], [ %storemerge43.i, %if.end.i ], [ undef, %if.else16.i ]
   %aSig0.2 = phi i64 [ %or12, %if.else ], [ %aSig0.0, %if.end.i ], [ undef, %if.else16.i ]
   %aExp.0 = phi i32 [ undef, %if.else ], [ %sub15.i, %if.end.i ], [ undef, %if.else16.i ]
   %shl2.i = shl i64 %aSig0.2, 15
   %shr.i = lshr i64 %aSig1.1, 49
   %or.i = or i64 %shl2.i, %shr.i
   tail call fastcc void @foo(%struct.0* noalias %agg.result, i8 signext 80, i8 zeroext undef, i32 %aExp.0, i64 %or.i, i64 undef)
   unreachable
 }
 
 attributes #0 = { norecurse nounwind }
 attributes #1 = { nounwind }

llvm/trunk/test/CodeGen/X86/pr13577.ll

 ; This would crash:
 ; https://llvm.org/bugs/show_bug.cgi?id=26070
 
 define float @pr26070() {
 ; CHECK-LABEL: pr26070:
 ; CHECK:       ## %bb.0:
 ; CHECK-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; CHECK-NEXT:    shufps {{.*#+}} xmm0 = xmm0[0,0,0,0]
-; CHECK-NEXT:    orps {{.*}}(%rip), %xmm0
 ; CHECK-NEXT:    retq
   %c = call float @copysignf(float 1.0, float undef) readnone
   ret float %c
 }
 
 declare float @copysignf(float, float)
 

llvm/trunk/test/CodeGen/X86/pr33960.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown -mattr=+avx | FileCheck %s --check-prefix=X86
 ; RUN: llc < %s -mtriple=x86_64-unknown -mattr=+avx | FileCheck %s --check-prefix=X64
 
 @b = external local_unnamed_addr global i32, align 4
 
 define void @PR33960() {
 ; X86-LABEL: PR33960:
 ; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $0, b
+; X86-NEXT:    movl $-1, b
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: PR33960:
 ; X64:       # %bb.0: # %entry
-; X64-NEXT:    movl $0, {{.*}}(%rip)
+; X64-NEXT:    movl $-1, {{.*}}(%rip)
 ; X64-NEXT:    retq
 entry:
   %tmp = insertelement <4 x i32> <i32 undef, i32 -7, i32 -3, i32 undef>, i32 -2, i32 3
   %predphi26 = insertelement <4 x i32> %tmp, i32 -7, i32 0
   %tmp1 = trunc <4 x i32> %predphi26 to <4 x i16>
   %tmp2 = icmp eq <4 x i16> %tmp1, zeroinitializer
   %tmp3 = icmp eq <4 x i32> undef, zeroinitializer
   %tmp4 = and <4 x i1> %tmp2, %tmp3

llvm/trunk/test/CodeGen/X86/undef-ops.ll

 ; CHECK-NEXT:    retq
   %r = mul i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @mul_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: mul_undef_rhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = mul <4 x i32> %x, undef
   ret <4 x i32> %r
 }
 
 define i32 @mul_undef_lhs(i32 %x) {
 ; CHECK-LABEL: mul_undef_lhs:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = mul i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @mul_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: mul_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = mul <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @sdiv_undef_rhs(i32 %x) {
 ; CHECK-LABEL: sdiv_undef_rhs:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = sdiv i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @sdiv_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: sdiv_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = sdiv <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @udiv_undef_rhs(i32 %x) {
 ; CHECK-LABEL: udiv_undef_rhs:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = udiv i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @udiv_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: udiv_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = udiv <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @srem_undef_rhs(i32 %x) {
 ; CHECK-LABEL: srem_undef_rhs:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = srem i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @srem_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: srem_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = srem <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @urem_undef_rhs(i32 %x) {
 ; CHECK-LABEL: urem_undef_rhs:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = urem i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @urem_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: urem_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = urem <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @ashr_undef_rhs(i32 %x) {
 ; CHECK-LABEL: ashr_undef_rhs:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl %edi, %eax
 ; CHECK-NEXT:    retq
   %r = ashr i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @ashr_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: ashr_undef_rhs_vec:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    retq
   %r = ashr <4 x i32> %x, undef
   ret <4 x i32> %r
 }
 
 define i32 @ashr_undef_lhs(i32 %x) {
 ; CHECK-LABEL: ashr_undef_lhs:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = ashr i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @ashr_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: ashr_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = ashr <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @lshr_undef_rhs(i32 %x) {
 ; CHECK-LABEL: lshr_undef_rhs:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = lshr i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @lshr_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: lshr_undef_rhs_vec:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = lshr i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @lshr_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: lshr_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = lshr <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @shl_undef_rhs(i32 %x) {
 ; CHECK-LABEL: shl_undef_rhs:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = shl i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @shl_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: shl_undef_rhs_vec:
 ; CHECK:       # %bb.0:

 ; CHECK-NEXT:    retq
   %r = shl i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @shl_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: shl_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = shl <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @and_undef_rhs(i32 %x) {
 ; CHECK-LABEL: and_undef_rhs:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = and i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @and_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: and_undef_rhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = and <4 x i32> %x, undef
   ret <4 x i32> %r
 }
 
 define i32 @and_undef_lhs(i32 %x) {
 ; CHECK-LABEL: and_undef_lhs:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    retq
   %r = and i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @and_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: and_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    xorps %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = and <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @or_undef_rhs(i32 %x) {
 ; CHECK-LABEL: or_undef_rhs:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl $-1, %eax
 ; CHECK-NEXT:    retq
   %r = or i32 %x, undef
   ret i32 %r
 }
 
 define <4 x i32> @or_undef_rhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: or_undef_rhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    pcmpeqd %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = or <4 x i32> %x, undef
   ret <4 x i32> %r
 }
 
 define i32 @or_undef_lhs(i32 %x) {
 ; CHECK-LABEL: or_undef_lhs:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl $-1, %eax
 ; CHECK-NEXT:    retq
   %r = or i32 undef, %x
   ret i32 %r
 }
 
 define <4 x i32> @or_undef_lhs_vec(<4 x i32> %x) {
 ; CHECK-LABEL: or_undef_lhs_vec:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    pcmpeqd %xmm0, %xmm0
 ; CHECK-NEXT:    retq
   %r = or <4 x i32> undef, %x
   ret <4 x i32> %r
 }
 
 define i32 @xor_undef_rhs(i32 %x) {
 ; CHECK-LABEL: xor_undef_rhs:
 ; CHECK:       # %bb.0: