[DAG] Cleanup unused node in SimplifySelectCC.

[niravd]

Summary

Delete temporarily constructed node uses for analysis after it's use,
holding onto original input nodes. Ideally this would be rewritten
without making nodes, but this appears relatively complex.

[spatel]

Can you describe what's happening within SimplifySetCC for the affected test?

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

18677 ↗

(On Diff #185851)

Could we replace this with a call to SelectionDAG::FoldSetCC()? That should only ever constant fold which AFAICT is what we're after.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

18677 ↗

(On Diff #185851)

Actually it looks like there's a tiny bit of commutation code (moves single constant to RHS) that probably needs pulling out of FoldSetCC into getNode - but that's trivial compared to the TLI.SimplifySetCC monster.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

18677 ↗

(On Diff #185851)

Confirmed, this gives the same result and is a lot cleaner:

// Determine if the condition we're dealing with is constant.
if (SDValue SCC = DAG.FoldSetCC(VT, N0, N1, CC, DL)) {
  if (auto *SCCC = dyn_cast<ConstantSDNode>(SCC)) {
    // fold select_cc true, x, y -> x
    // fold select_cc false, x, y -> y
    return !SCCC->isNullValue() ? N2 : N3;
  }
}

[RKSimon]

@niravd Are you still looking at this?

[niravd]

Copy over Simon's suggestion.

[niravd]

It fell off the end of my work queue, but yes.

There's also a natural followup replacing another instance of SimplifySetCC, which seems to have similar wins that I'll put up in a bit.

In D57921#1433138, @RKSimon wrote:

@niravd Are you still looking at this?

[RKSimon]

LGTM - cheers

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

SDValue DAGCombiner::SimplifySelectCC(const SDLoc &DL, SDValue N0, SDValue N1,
 
   EVT CmpOpVT = N0.getValueType();
   EVT VT = N2.getValueType();
   auto *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
   auto *N2C = dyn_cast<ConstantSDNode>(N2.getNode());
   auto *N3C = dyn_cast<ConstantSDNode>(N3.getNode());
 
   // Determine if the condition we're dealing with is constant.
-  SDValue SCC = SimplifySetCC(getSetCCResultType(CmpOpVT), N0, N1, CC, DL,
-                              false);
-  if (SCC.getNode()) AddToWorklist(SCC.getNode());
-
-  if (auto *SCCC = dyn_cast_or_null<ConstantSDNode>(SCC.getNode())) {
+  if (SDValue SCC = DAG.FoldSetCC(VT, N0, N1, CC, DL)) {
+    AddToWorklist(SCC.getNode());
+    if (auto *SCCC = dyn_cast<ConstantSDNode>(SCC)) {
       // fold select_cc true, x, y -> x
       // fold select_cc false, x, y -> y
-    return !SCCC->isNullValue() ? N2 : N3;
+      return !(SCCC->isNullValue()) ? N2 : N3;
+    }
   }
 
   if (SDValue V =
           convertSelectOfFPConstantsToLoadOffset(DL, N0, N1, N2, N3, CC))
     return V;
 
   if (SDValue V = foldSelectCCToShiftAnd(DL, N0, N1, N2, N3, CC))
     return V;

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

 ; X32-NEXT:    xorl $1, %edx
 ; X32-NEXT:    xorl %eax, %eax
 ; X32-NEXT:    orl %ecx, %edx
 ; X32-NEXT:    setne %al
 ; X32-NEXT:    retl
 ;
 ; X64-LIN-LABEL: PR17487:
 ; X64-LIN:       # %bb.0:
-; X64-LIN-NEXT:    movd %edi, %xmm0
-; X64-LIN-NEXT:    pextrw $0, %xmm0, %eax
+; X64-LIN-NEXT:    movl %edi, %eax
 ; X64-LIN-NEXT:    andl $1, %eax
 ; X64-LIN-NEXT:    retq
 ;
 ; X64-WIN-LABEL: PR17487:
 ; X64-WIN:       # %bb.0:
-; X64-WIN-NEXT:    andb $1, %cl
 ; X64-WIN-NEXT:    movzbl %cl, %eax
+; X64-WIN-NEXT:    andl $1, %eax
 ; X64-WIN-NEXT:    retq
   %tmp = insertelement <2 x i1> undef, i1 %tobool, i32 1
   %tmp1 = zext <2 x i1> %tmp to <2 x i64>
   %tmp2 = xor <2 x i64> %tmp1, <i64 1, i64 1>
   %tmp3 = extractelement <2 x i64> %tmp2, i32 1
   %add = add nsw i64 0, %tmp3
   %cmp6 = icmp ne i64 %add, 1
   %conv7 = zext i1 %cmp6 to i32