[DAG] Aggressively cleanup dangling node in CombineZExtLogicopShiftLoad.

[niravd]

Summary

While dangling nodes will eventually be pruned when they are
considered, leaving them disables combines requiring single-use.

[spatel]

Can you describe what's happening in these tests? Should we consider the x86 diffs (inc -> add) regressions?
I'm trying to figure out how I managed to get the same set of diffs in D57516. :)

[niravd]

Hmm , weird. I've been looking at reasons why D57367 is doing so much when I found the case generating dangling node. I _was_ a bit surprised that this causes that regression, but it must be the case that dangling nodes persist to ISel and prevent folding. Presumably your change allowed CombineZExtLogicOpShiftLoad to trigger.

In any case, this is obvious cleanup and rebasing past your revert this is NFCish as expected again. I'll land this presently.

[RKSimon]

@niravd was committing this intentional?

[spatel]

I see what happened here.
This patch was based on something after rL352766, so it picked up the test diffs that were mistakenly omitted by the first rev of D57516.
On its own, this patch shows no test diffs, so Nirav proceeded with committing this after rL352769.

D57516 has *no* interaction with this patch AFAICT. That makes a lot more sense than both patches tickling exactly the same set of regression tests.

I have a separate x86 patch in progress to avoid the add->inc seen in the other patch.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::CombineZExtLogicopShiftLoad(SDNode *N) {
   APInt Mask = cast<ConstantSDNode>(N0.getOperand(1))->getAPIntValue();
   Mask = Mask.zext(VT.getSizeInBits());
   SDLoc DL0(N0);
   SDValue And = DAG.getNode(N0.getOpcode(), DL0, VT, Shift,
                             DAG.getConstant(Mask, DL0, VT));
 
   ExtendSetCCUses(SetCCs, N1.getOperand(0), ExtLoad, ISD::ZERO_EXTEND);
   CombineTo(N, And);
+
   if (SDValue(Load, 0).hasOneUse()) {
     DAG.ReplaceAllUsesOfValueWith(SDValue(Load, 1), ExtLoad.getValue(1));
   } else {
     SDValue Trunc = DAG.getNode(ISD::TRUNCATE, SDLoc(Load),
                                 Load->getValueType(0), ExtLoad);
     CombineTo(Load, Trunc, ExtLoad.getValue(1));
   }
+
+  // N0 is dead at this point.
+  recursivelyDeleteUnusedNodes(N0.getNode());
+
   return SDValue(N,0); // Return N so it doesn't get rechecked!
 }
 
 /// If we're narrowing or widening the result of a vector select and the final
 /// size is the same size as a setcc (compare) feeding the select, then try to
 /// apply the cast operation to the select's operands because matching vector
 /// sizes for a select condition and other operands should be more efficient.
 SDValue DAGCombiner::matchVSelectOpSizesWithSetCC(SDNode *Cast) {

llvm/test/CodeGen/Hexagon/swp-epilog-phi5.ll

 ; RUN: llc -march=hexagon < %s | FileCheck %s
 
 ; Test that we use the correct name in an epilog phi for a phi value
 ; that is defined for the last time in the kernel. Previously, we
 ; used the value from kernel loop definition, but we really need
 ; to use the value from the Phi in the kernel instead.
 
 ; In this test case, the second loop is pipelined, block b5.
 
 ; CHECK: loop0
 ; CHECK: [[REG0:r([0-9]+)]] += mpyi
 ; CHECK-NOT: r{{[0-9]+}} += add([[REG0]],#8)
-; CHECK: endloop1
+; CHECK: endloop0
 
 %s.0 = type { %s.1*, %s.4*, %s.7*, i8*, i8, i32, %s.8*, i32, i32, i32, i8, i8, i32, i32, double, i8, i8, i8, i8, i8, i8, i8, i8, i32, i8, i8, i8, i32, i32, i32, i32, i32, i32, i8**, i32, i32, i32, i32, i32, [64 x i32]*, [4 x %s.9*], [4 x %s.10*], [4 x %s.10*], i32, %s.23*, i8, i8, [16 x i8], [16 x i8], [16 x i8], i32, i8, i8, i8, i8, i16, i16, i8, i8, i8, %s.11*, i32, i32, i32, i32, i8*, i32, [4 x %s.23*], i32, i32, i32, [10 x i32], i32, i32, i32, i32, i32, %s.12*, %s.13*, %s.14*, %s.15*, %s.16*, %s.17*, %s.18*, %s.19*, %s.20*, %s.21*, %s.22* }
 %s.1 = type { void (%s.2*)*, void (%s.2*, i32)*, void (%s.2*)*, void (%s.2*, i8*)*, void (%s.2*)*, i32, %s.3, i32, i32, i8**, i32, i8**, i32, i32 }
 %s.2 = type { %s.1*, %s.4*, %s.7*, i8*, i8, i32 }
 %s.3 = type { [8 x i32], [48 x i8] }
 %s.4 = type { i8* (%s.2*, i32, i32)*, i8* (%s.2*, i32, i32)*, i8** (%s.2*, i32, i32, i32)*, [64 x i16]** (%s.2*, i32, i32, i32)*, %s.5* (%s.2*, i32, i8, i32, i32, i32)*, %s.6* (%s.2*, i32, i8, i32, i32, i32)*, {}*, i8** (%s.2*, %s.5*, i32, i32, i8)*, [64 x i16]** (%s.2*, %s.6*, i32, i32, i8)*, void (%s.2*, i32)*, {}*, i32, i32 }
 %s.5 = type opaque
 %s.6 = type opaque

llvm/test/CodeGen/X86/copy-eflags.ll

 ; being clobbered by the second increment.
 define i32 @test1() nounwind {
 ; X32-LABEL: test1:
 ; X32:       # %bb.0: # %entry
 ; X32-NEXT:    movb b, %cl
 ; X32-NEXT:    movl %ecx, %eax
 ; X32-NEXT:    incb %al
 ; X32-NEXT:    movb %al, b
-; X32-NEXT:    incl c
-; X32-NEXT:    sete %dl
+; X32-NEXT:    addl $1, c
+; X32-NEXT:    setb %dl
 ; X32-NEXT:    movb a, %ah
 ; X32-NEXT:    movb %ah, %ch
 ; X32-NEXT:    incb %ch
 ; X32-NEXT:    cmpb %cl, %ah
 ; X32-NEXT:    sete d
 ; X32-NEXT:    movb %ch, a
 ; X32-NEXT:    testb %dl, %dl
 ; X32-NEXT:    jne .LBB0_2
 ; X32-NEXT:  # %bb.1: # %if.then
 ; X32-NEXT:    movsbl %al, %eax
 ; X32-NEXT:    pushl %eax
 ; X32-NEXT:    calll external
 ; X32-NEXT:    addl $4, %esp
 ; X32-NEXT:  .LBB0_2: # %if.end
 ; X32-NEXT:    xorl %eax, %eax
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: test1:
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    movb {{.*}}(%rip), %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    incb %al
 ; X64-NEXT:    movb %al, {{.*}}(%rip)
-; X64-NEXT:    incl {{.*}}(%rip)
-; X64-NEXT:    sete %sil
+; X64-NEXT:    addl $1, {{.*}}(%rip)
+; X64-NEXT:    setb %sil
 ; X64-NEXT:    movb {{.*}}(%rip), %cl
 ; X64-NEXT:    movl %ecx, %edx
 ; X64-NEXT:    incb %dl
 ; X64-NEXT:    cmpb %dil, %cl
 ; X64-NEXT:    sete {{.*}}(%rip)
 ; X64-NEXT:    movb %dl, {{.*}}(%rip)
 ; X64-NEXT:    testb %sil, %sil
 ; X64-NEXT:    jne .LBB0_2

 }
 
 ; Preserve increment flags across a call.
 define i32 @test2(i32* %ptr) nounwind {
 ; X32-LABEL: test2:
 ; X32:       # %bb.0: # %entry
 ; X32-NEXT:    pushl %ebx
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X32-NEXT:    incl (%eax)
-; X32-NEXT:    setne %bl
+; X32-NEXT:    addl $1, (%eax)
+; X32-NEXT:    setae %bl
 ; X32-NEXT:    pushl $42
 ; X32-NEXT:    calll external
 ; X32-NEXT:    addl $4, %esp
 ; X32-NEXT:    testb %bl, %bl
-; X32-NEXT:    je .LBB1_1
-; X32-NEXT:  # %bb.2: # %else
-; X32-NEXT:    xorl %eax, %eax
+; X32-NEXT:    jne .LBB1_2
+; X32-NEXT:  # %bb.1: # %then
+; X32-NEXT:    movl $64, %eax
 ; X32-NEXT:    popl %ebx
 ; X32-NEXT:    retl
-; X32-NEXT:  .LBB1_1: # %then
-; X32-NEXT:    movl $64, %eax
+; X32-NEXT:  .LBB1_2: # %else
+; X32-NEXT:    xorl %eax, %eax
 ; X32-NEXT:    popl %ebx
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: test2:
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    pushq %rbx
-; X64-NEXT:    incl (%rdi)
-; X64-NEXT:    setne %bl
+; X64-NEXT:    addl $1, (%rdi)
+; X64-NEXT:    setae %bl
 ; X64-NEXT:    movl $42, %edi
 ; X64-NEXT:    callq external
 ; X64-NEXT:    testb %bl, %bl
-; X64-NEXT:    je .LBB1_1
-; X64-NEXT:  # %bb.2: # %else
-; X64-NEXT:    xorl %eax, %eax
+; X64-NEXT:    jne .LBB1_2
+; X64-NEXT:  # %bb.1: # %then
+; X64-NEXT:    movl $64, %eax
 ; X64-NEXT:    popq %rbx
 ; X64-NEXT:    retq
-; X64-NEXT:  .LBB1_1: # %then
-; X64-NEXT:    movl $64, %eax
+; X64-NEXT:  .LBB1_2: # %else
+; X64-NEXT:    xorl %eax, %eax
 ; X64-NEXT:    popq %rbx
 ; X64-NEXT:    retq
 entry:
   %val = load i32, i32* %ptr
   %inc = add i32 %val, 1
   store i32 %inc, i32* %ptr
   %cmp = icmp eq i32 %inc, 0
   call void @external(i32 42)

 ; is tricky because we can only do this from a leaf function, and so we have to
 ; use volatile stores similar to test1 to force the save and restore of
 ; a condition without calling another function. We then set up subsequent calls
 ; in tail position.
 define void @test_tail_call(i32* %ptr) nounwind optsize {
 ; X32-LABEL: test_tail_call:
 ; X32:       # %bb.0: # %entry
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X32-NEXT:    incl (%eax)
-; X32-NEXT:    setne %al
-; X32-NEXT:    incb a
-; X32-NEXT:    sete d
+; X32-NEXT:    addl $1, (%eax)
+; X32-NEXT:    setae %al
+; X32-NEXT:    addb $1, a
+; X32-NEXT:    setb d
 ; X32-NEXT:    testb %al, %al
 ; X32-NEXT:    jne external_b # TAILCALL
 ; X32-NEXT:  # %bb.1: # %then
 ; X32-NEXT:    jmp external_a # TAILCALL
 ;
 ; X64-LABEL: test_tail_call:
 ; X64:       # %bb.0: # %entry
-; X64-NEXT:    incl (%rdi)
-; X64-NEXT:    setne %al
-; X64-NEXT:    incb {{.*}}(%rip)
-; X64-NEXT:    sete {{.*}}(%rip)
+; X64-NEXT:    addl $1, (%rdi)
+; X64-NEXT:    setae %al
+; X64-NEXT:    addb $1, {{.*}}(%rip)
+; X64-NEXT:    setb {{.*}}(%rip)
 ; X64-NEXT:    testb %al, %al
 ; X64-NEXT:    jne external_b # TAILCALL
 ; X64-NEXT:  # %bb.1: # %then
 ; X64-NEXT:    jmp external_a # TAILCALL
 entry:
   %val = load i32, i32* %ptr
   %inc = add i32 %val, 1
   store i32 %inc, i32* %ptr

llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll

 @M = external global i8
 
 declare i32 @bar(i64)
 
 define i1 @plus_one() nounwind {
 ; CHECK32-LABEL: plus_one:
 ; CHECK32:       # %bb.0: # %entry
 ; CHECK32-NEXT:    movb M, %al
-; CHECK32-NEXT:    incl L
-; CHECK32-NEXT:    jne .LBB0_2
+; CHECK32-NEXT:    addl $1, L
+; CHECK32-NEXT:    jae .LBB0_2
 ; CHECK32-NEXT:  # %bb.1: # %entry
 ; CHECK32-NEXT:    andb $8, %al
 ; CHECK32-NEXT:    je .LBB0_2
 ; CHECK32-NEXT:  # %bb.3: # %exit2
 ; CHECK32-NEXT:    xorl %eax, %eax
 ; CHECK32-NEXT:    retl
 ; CHECK32-NEXT:  .LBB0_2: # %exit
 ; CHECK32-NEXT:    movb $1, %al
 ; CHECK32-NEXT:    retl
 ;
 ; CHECK64-LABEL: plus_one:
 ; CHECK64:       # %bb.0: # %entry
 ; CHECK64-NEXT:    movb {{.*}}(%rip), %al
-; CHECK64-NEXT:    incl {{.*}}(%rip)
-; CHECK64-NEXT:    jne .LBB0_2
+; CHECK64-NEXT:    addl $1, {{.*}}(%rip)
+; CHECK64-NEXT:    jae .LBB0_2
 ; CHECK64-NEXT:  # %bb.1: # %entry
 ; CHECK64-NEXT:    andb $8, %al
 ; CHECK64-NEXT:    je .LBB0_2
 ; CHECK64-NEXT:  # %bb.3: # %exit2
 ; CHECK64-NEXT:    xorl %eax, %eax
 ; CHECK64-NEXT:    retq
 ; CHECK64-NEXT:  .LBB0_2: # %exit
 ; CHECK64-NEXT:    movb $1, %al

llvm/test/CodeGen/X86/rd-mod-wr-eflags.ll

 return:                                           ; preds = %if.end4, %if.end, %entry
   ret void
 }
 
 define void @example_inc(%struct.obj2* %o) nounwind uwtable ssp {
 ; 64 bit inc
 ; CHECK-LABEL: example_inc:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    incq (%rdi)
-; CHECK-NEXT:    jne .LBB4_4
+; CHECK-NEXT:    addq $1, (%rdi)
+; CHECK-NEXT:    jae .LBB4_4
 ; CHECK-NEXT:  # %bb.1: # %if.end
-; CHECK-NEXT:    incl 8(%rdi)
-; CHECK-NEXT:    jne .LBB4_4
+; CHECK-NEXT:    addl $1, 8(%rdi)
+; CHECK-NEXT:    jae .LBB4_4
 ; CHECK-NEXT:  # %bb.2: # %if.end1
-; CHECK-NEXT:    incw 12(%rdi)
-; CHECK-NEXT:    jne .LBB4_4
+; CHECK-NEXT:    addw $1, 12(%rdi)
+; CHECK-NEXT:    jae .LBB4_4
 ; CHECK-NEXT:  # %bb.3: # %if.end2
-; CHECK-NEXT:    incb 14(%rdi)
-; CHECK-NEXT:    je .LBB4_5
+; CHECK-NEXT:    addb $1, 14(%rdi)
+; CHECK-NEXT:    jae .LBB4_4
+; CHECK-NEXT:  # %bb.5: # %if.end4
+; CHECK-NEXT:    jmp other # TAILCALL
 ; CHECK-NEXT:  .LBB4_4: # %return
 ; CHECK-NEXT:    retq
-; CHECK-NEXT:  .LBB4_5: # %if.end4
-; CHECK-NEXT:    jmp other # TAILCALL
 entry:
   %s64 = getelementptr inbounds %struct.obj2, %struct.obj2* %o, i64 0, i32 0
   %0 = load i64, i64* %s64, align 8
   %inc = add i64 %0, 1
   store i64 %inc, i64* %s64, align 8
   %tobool = icmp eq i64 %inc, 0
   br i1 %tobool, label %if.end, label %return
 

llvm/test/CodeGen/X86/reverse_branches.ll

 ; CHECK-NEXT:  LBB0_12: ## %do.body.i
 ; CHECK-NEXT:    ## Parent Loop BB0_9 Depth=1
 ; CHECK-NEXT:    ## Parent Loop BB0_10 Depth=2
 ; CHECK-NEXT:    ## => This Inner Loop Header: Depth=3
 ; CHECK-NEXT:    cmpb $120, 1000(%rdx,%rbp)
 ; CHECK-NEXT:    je LBB0_14
 ; CHECK-NEXT:  ## %bb.13: ## %do.cond.i
 ; CHECK-NEXT:    ## in Loop: Header=BB0_12 Depth=3
-; CHECK-NEXT:    incq %rbp
-; CHECK-NEXT:    jne LBB0_12
+; CHECK-NEXT:    addq $1, %rbp
+; CHECK-NEXT:    jae LBB0_12
 ; CHECK-NEXT:    jmp LBB0_5
 ; CHECK-NEXT:    .p2align 4, 0x90
 ; CHECK-NEXT:  LBB0_14: ## %exit
 ; CHECK-NEXT:    ## in Loop: Header=BB0_10 Depth=2
 ; CHECK-NEXT:    addq %rsi, %rbp
 ; CHECK-NEXT:    incq %rdi
 ; CHECK-NEXT:    decq %rsi
 ; CHECK-NEXT:    addq $1001, %rdx ## imm = 0x3E9