[DAGCombiner] Set the right SDLoc on extended SETCC uses

[vsk]

Summary

ExtendSetCCUses updates SETCC nodes which use a load (OriginalLoad) to
reflect a simplification to the load (ExtLoad).

Based on my reading, ExtendSetCCUses may create new nodes to extend a
constant attached to a SETCC. It also creates fresh SETCC nodes which
refer to any updated operands.

ISTM that the location applied to the new constant and SETCC nodes
should be the same as the location of the ExtLoad.

This was suggested by Adrian in https://reviews.llvm.org/D45995.

[niravd]

This patch looks fine modulo the comment made but that can be pushed into another patch.

LGTM.

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

public:
 
     /// Convenience wrapper around TargetLowering::getSetCCResultType
     EVT getSetCCResultType(EVT VT) const {
       return TLI.getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), VT);
     }
 
     void ExtendSetCCUses(const SmallVectorImpl<SDNode *> &SetCCs,
                          SDValue OrigLoad, SDValue ExtLoad,
-                         const SDLoc &DL,
                          ISD::NodeType ExtType);
   };
 
 /// This class is a DAGUpdateListener that removes any deleted
 /// nodes from the worklist.
 class WorklistRemover : public SelectionDAG::DAGUpdateListener {
   DAGCombiner &DC;
 
if (BothLiveOut)
       // a good reason for the transformation.
       return ExtendNodes.size();
   }
   return true;
 }
 
 void DAGCombiner::ExtendSetCCUses(const SmallVectorImpl<SDNode *> &SetCCs,
                                   SDValue OrigLoad, SDValue ExtLoad,
-                                  const SDLoc &DL, ISD::NodeType ExtType) {
+                                  ISD::NodeType ExtType) {
   // Extend SetCC uses if necessary.
-  for (unsigned i = 0, e = SetCCs.size(); i != e; ++i) {
-    SDNode *SetCC = SetCCs[i];
+  SDLoc DL(ExtLoad);
+  for (SDNode *SetCC : SetCCs) {
     SmallVector<SDValue, 4> Ops;
 
     for (unsigned j = 0; j != 2; ++j) {
       SDValue SOp = SetCC->getOperand(j);
       if (SOp == OrigLoad)
         Ops.push_back(ExtLoad);
       else
         Ops.push_back(DAG.getNode(ExtType, DL, ExtLoad->getValueType(0), SOp));
SDValue DAGCombiner::CombineExtLoad(SDNode *N) {
   AddToWorklist(NewChain.getNode());
 
   CombineTo(N, NewValue);
 
   // Replace uses of the original load (before extension)
   // with a truncate of the concatenated sextloaded vectors.
   SDValue Trunc =
       DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), NewValue);
-  ExtendSetCCUses(SetCCs, N0, NewValue, DL,
-                  (ISD::NodeType)N->getOpcode());
+  ExtendSetCCUses(SetCCs, N0, NewValue, (ISD::NodeType)N->getOpcode());
   CombineTo(N0.getNode(), Trunc, NewChain);
   return SDValue(N, 0); // Return N so it doesn't get rechecked!
 }
 
 // fold (zext (and/or/xor (shl/shr (load x), cst), cst)) ->
 //      (and/or/xor (shl/shr (zextload x), (zext cst)), (zext cst))
 SDValue DAGCombiner::CombineZExtLogicopShiftLoad(SDNode *N) {
   assert(N->getOpcode() == ISD::ZERO_EXTEND);
SDValue Shift = DAG.getNode(N1.getOpcode(), DL1, VT, ExtLoad,
                               N1.getOperand(1));
 
   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, SDLoc(Load),
-                  ISD::ZERO_EXTEND);
+  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));
   }

 }
 
 // fold ([s|z]ext (load x)) -> ([s|z]ext (truncate ([s|z]extload x)))
 // Only generate vector extloads when 1) they're legal, and 2) they are
 // deemed desirable by the target.
 static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
                                   const TargetLowering &TLI, EVT VT,
                                   bool LegalOperations, SDNode *N, SDValue N0,
-                                  SDLoc DL, ISD::LoadExtType ExtLoadType,
+                                  ISD::LoadExtType ExtLoadType,
                                   ISD::NodeType ExtOpc) {
   if (!ISD::isNON_EXTLoad(N0.getNode()) ||
       !ISD::isUNINDEXEDLoad(N0.getNode()) ||
       ((LegalOperations || VT.isVector() ||
         cast<LoadSDNode>(N0)->isVolatile()) &&
        !TLI.isLoadExtLegal(ExtLoadType, VT, N0.getValueType())))
     return {};
 
   bool DoXform = true;
   SmallVector<SDNode *, 4> SetCCs;
   if (!N0.hasOneUse())
     DoXform = ExtendUsesToFormExtLoad(VT, N, N0, ExtOpc, SetCCs, TLI);
   if (VT.isVector())
     DoXform &= TLI.isVectorLoadExtDesirable(SDValue(N, 0));
   if (!DoXform)
     return {};
 
   LoadSDNode *LN0 = cast<LoadSDNode>(N0);
   SDValue ExtLoad = DAG.getExtLoad(ExtLoadType, SDLoc(LN0), VT, LN0->getChain(),
                                    LN0->getBasePtr(), N0.getValueType(),
                                    LN0->getMemOperand());
-  Combiner.ExtendSetCCUses(SetCCs, N0, ExtLoad, DL, ExtOpc);
+  Combiner.ExtendSetCCUses(SetCCs, N0, ExtLoad, ExtOpc);
   // If the load value is used only by N, replace it via CombineTo N.
   bool NoReplaceTrunc = SDValue(LN0, 0).hasOneUse();
   Combiner.CombineTo(N, ExtLoad);
   if (NoReplaceTrunc) {
     DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1));
   } else {
     SDValue Trunc =
         DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad);
if (!LegalOperations || TLI.isOperationLegal(ISD::SIGN_EXTEND_INREG,
         Op = DAG.getNode(ISD::ANY_EXTEND, SDLoc(N0), VT, Op);
       else if (OpBits > DestBits)
         Op = DAG.getNode(ISD::TRUNCATE, SDLoc(N0), VT, Op);
       return DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, VT, Op,
                          DAG.getValueType(N0.getValueType()));
     }
   }
 
-  // Try to fold (sext (load x)) to a smaller sextload.
+  // Try to simplify (sext (load x)).
   if (SDValue foldedExt =
-          tryToFoldExtOfLoad(DAG, *this, TLI, VT, LegalOperations, N, N0, DL,
+          tryToFoldExtOfLoad(DAG, *this, TLI, VT, LegalOperations, N, N0,
                              ISD::SEXTLOAD, ISD::SIGN_EXTEND))
     return foldedExt;
 
   // fold (sext (load x)) to multiple smaller sextloads.
   // Only on illegal but splittable vectors.
   if (SDValue ExtLoad = CombineExtLoad(N))
     return ExtLoad;
 
if (TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, MemVT) &&
         SDValue ExtLoad = DAG.getExtLoad(ISD::SEXTLOAD, SDLoc(LN00), VT,
                                          LN00->getChain(), LN00->getBasePtr(),
                                          LN00->getMemoryVT(),
                                          LN00->getMemOperand());
         APInt Mask = cast<ConstantSDNode>(N0.getOperand(1))->getAPIntValue();
         Mask = Mask.sext(VT.getSizeInBits());
         SDValue And = DAG.getNode(N0.getOpcode(), DL, VT,
                                   ExtLoad, DAG.getConstant(Mask, DL, VT));
-        ExtendSetCCUses(SetCCs, N0.getOperand(0), ExtLoad, DL,
-                        ISD::SIGN_EXTEND);
+        ExtendSetCCUses(SetCCs, N0.getOperand(0), ExtLoad, ISD::SIGN_EXTEND);
         bool NoReplaceTruncAnd = !N0.hasOneUse();
         bool NoReplaceTrunc = SDValue(LN00, 0).hasOneUse();
         CombineTo(N, And);
         // If N0 has multiple uses, change other uses as well.
         if (NoReplaceTruncAnd) {
           SDValue TruncAnd =
               DAG.getNode(ISD::TRUNCATE, DL, N0.getValueType(), And);
           CombineTo(N0.getNode(), TruncAnd);
if (N0.getOpcode() == ISD::AND &&
     SDLoc DL(N);
     return DAG.getNode(ISD::AND, DL, VT,
                        X, DAG.getConstant(Mask, DL, VT));
   }
 
   // Try to simplify (zext (load x)).
   if (SDValue foldedExt =
           tryToFoldExtOfLoad(DAG, *this, TLI, VT, LegalOperations, N, N0,
-                             SDLoc(N), ISD::ZEXTLOAD, ISD::ZERO_EXTEND))
+                             ISD::ZEXTLOAD, ISD::ZERO_EXTEND))
     return foldedExt;
 
   // fold (zext (load x)) to multiple smaller zextloads.
   // Only on illegal but splittable vectors.
   if (SDValue ExtLoad = CombineExtLoad(N))
     return ExtLoad;
 
   // fold (zext (and/or/xor (load x), cst)) ->
if (TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT) &&
                                          LN00->getChain(), LN00->getBasePtr(),
                                          LN00->getMemoryVT(),
                                          LN00->getMemOperand());
         APInt Mask = cast<ConstantSDNode>(N0.getOperand(1))->getAPIntValue();
         Mask = Mask.zext(VT.getSizeInBits());
         SDLoc DL(N);
         SDValue And = DAG.getNode(N0.getOpcode(), DL, VT,
                                   ExtLoad, DAG.getConstant(Mask, DL, VT));
-        ExtendSetCCUses(SetCCs, N0.getOperand(0), ExtLoad, DL,
-                        ISD::ZERO_EXTEND);
+        ExtendSetCCUses(SetCCs, N0.getOperand(0), ExtLoad, ISD::ZERO_EXTEND);
         bool NoReplaceTruncAnd = !N0.hasOneUse();
         bool NoReplaceTrunc = SDValue(LN00, 0).hasOneUse();
         CombineTo(N, And);
         // If N0 has multiple uses, change other uses as well.
         if (NoReplaceTruncAnd) {
           SDValue TruncAnd =
               DAG.getNode(ISD::TRUNCATE, DL, N0.getValueType(), And);
           CombineTo(N0.getNode(), TruncAnd);
if (!N0.hasOneUse())
       DoXform = ExtendUsesToFormExtLoad(VT, N, N0, ISD::ANY_EXTEND, SetCCs,
                                         TLI);
     if (DoXform) {
       LoadSDNode *LN0 = cast<LoadSDNode>(N0);
       SDValue ExtLoad = DAG.getExtLoad(ISD::EXTLOAD, SDLoc(N), VT,
                                        LN0->getChain(),
                                        LN0->getBasePtr(), N0.getValueType(),
                                        LN0->getMemOperand());
-      ExtendSetCCUses(SetCCs, N0, ExtLoad, SDLoc(N),
-                      ISD::ANY_EXTEND);
+      ExtendSetCCUses(SetCCs, N0, ExtLoad, ISD::ANY_EXTEND);
       // If the load value is used only by N, replace it via CombineTo N.
       bool NoReplaceTrunc = N0.hasOneUse();
       CombineTo(N, ExtLoad);
       if (NoReplaceTrunc) {
         DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1));
       } else {
         SDValue Trunc = DAG.getNode(ISD::TRUNCATE, SDLoc(N0),
                                     N0.getValueType(), ExtLoad);

llvm/trunk/test/CodeGen/X86/extend-set-cc-uses-dbg.ll

+; RUN: llc < %s -O0 -stop-after=livedebugvalues -o - | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.13.0"
+
+define void @foo(i32* %p) !dbg !4 {
+bb:
+  %tmp = load i32, i32* %p, align 4, !dbg !7
+  ; CHECK: $eax = MOV32rm killed {{.*}} $rdi, {{.*}} debug-location !7 :: (load 4 from %ir.p)
+  ; CHECK-NEXT: $edi = MOV32rr killed $eax, implicit-def $rdi, debug-location !7
+  ; CHECK-NEXT: $rcx = MOV64rr $rdi, debug-location !7
+
+  switch i32 %tmp, label %bb7 [
+    i32 0, label %bb1
+    i32 1, label %bb2
+    i32 2, label %bb3
+    i32 3, label %bb4
+  ], !dbg !8
+
+bb1:                                              ; preds = %bb
+  unreachable
+
+bb2:                                              ; preds = %bb
+  unreachable
+
+bb3:                                              ; preds = %bb
+  unreachable
+
+bb4:                                              ; preds = %bb
+  unreachable
+
+bb7:                                              ; preds = %bb
+  ret void, !dbg !9
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 7.0.0 (trunk 330770) (llvm/trunk 330769)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly)
+!1 = !DIFile(filename: "/Users/vsk/src/llvm.org-master/llvm/lib/Demangle/ItaniumDemangle.cpp", directory: "/Users/vsk/src/builds/llvm.org-master-RA-stage2")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{i32 7, !"PIC Level", i32 2}
+!4 = distinct !DISubprogram(name: "printLeft", scope: !1, file: !1, line: 1306, type: !5, isLocal: true, isDefinition: true, scopeLine: 1306, flags: DIFlagPrototyped, isOptimized: true, unit: !0)
+!5 = !DISubroutineType(types: !6)
+!6 = !{}
+!7 = !DILocation(line: 1307, column: 13, scope: !4)
+!8 = !DILocation(line: 1307, column: 5, scope: !4)
+!9 = !DILocation(line: 1327, column: 3, scope: !4)

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

 ; X86-O0-NEXT:    andb $1, %dil
 ; X86-O0-NEXT:    movzbl %dil, %eax
 ; X86-O0-NEXT:    movl %eax, -{{[0-9]+}}(%rsp)
 ; X86-O0-NEXT:    retq
 ;
 ; X64-LABEL: foo:
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    movzbl {{.*}}(%rip), %eax
-; X64-NEXT:    testb %al, %al
-; X64-NEXT:    setne -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    xorl %ecx, %ecx
 ; X64-NEXT:    testl %eax, %eax
 ; X64-NEXT:    setne %cl
+; X64-NEXT:    testb %al, %al
+; X64-NEXT:    setne -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    xorl %edx, %edx
 ; X64-NEXT:    cmpl %eax, %ecx
 ; X64-NEXT:    setle %dl
 ; X64-NEXT:    movl %edx, -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    retq
 ;
 ; 686-O0-LABEL: foo:
 ; 686-O0:       # %bb.0: # %entry

 ; X86-O0-NEXT:    movzbl %dl, %esi
 ; X86-O0-NEXT:    movl %esi, %eax
 ; X86-O0-NEXT:    movq %rax, _ZN8struct_210member_2_0E
 ; X86-O0-NEXT:    retq
 ;
 ; X64-LABEL: f1:
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    movslq {{.*}}(%rip), %rax
+; X64-NEXT:    movabsq $-8381627093, %rcx # imm = 0xFFFFFFFE0C6A852B
+; X64-NEXT:    cmpq %rcx, %rax
+; X64-NEXT:    setne -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    xorl %ecx, %ecx
 ; X64-NEXT:    cmpq $-1, %rax
 ; X64-NEXT:    sete %cl
-; X64-NEXT:    movabsq $-8381627093, %rdx # imm = 0xFFFFFFFE0C6A852B
-; X64-NEXT:    cmpq %rdx, %rax
-; X64-NEXT:    setne -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    xorl %edx, %edx
 ; X64-NEXT:    cmpl $-1, %eax
 ; X64-NEXT:    sete %dl
 ; X64-NEXT:    addq $7093, %rax # imm = 0x1BB5
 ; X64-NEXT:    xorl %esi, %esi
 ; X64-NEXT:    cmpq %rax, %rdx
 ; X64-NEXT:    setg %sil
 ; X64-NEXT:    movq %rsi, {{.*}}(%rip)

 ; 686-NEXT:    .cfi_def_cfa_offset 8
 ; 686-NEXT:    pushl %esi
 ; 686-NEXT:    .cfi_def_cfa_offset 12
 ; 686-NEXT:    subl $1, %esp
 ; 686-NEXT:    .cfi_def_cfa_offset 13
 ; 686-NEXT:    .cfi_offset %esi, -12
 ; 686-NEXT:    .cfi_offset %edi, -8
 ; 686-NEXT:    movl var_5, %edx
+; 686-NEXT:    movl %edx, %eax
+; 686-NEXT:    xorl $208307499, %eax # imm = 0xC6A852B
 ; 686-NEXT:    movl %edx, %esi
 ; 686-NEXT:    sarl $31, %esi
+; 686-NEXT:    movl %esi, %ecx
+; 686-NEXT:    xorl $-2, %ecx
+; 686-NEXT:    orl %eax, %ecx
+; 686-NEXT:    setne (%esp)
 ; 686-NEXT:    movl %edx, %ecx
 ; 686-NEXT:    andl %esi, %ecx
 ; 686-NEXT:    xorl %eax, %eax
 ; 686-NEXT:    cmpl $-1, %ecx
 ; 686-NEXT:    sete %al
-; 686-NEXT:    movl %edx, %ecx
-; 686-NEXT:    xorl $208307499, %ecx # imm = 0xC6A852B
-; 686-NEXT:    movl %esi, %edi
-; 686-NEXT:    xorl $-2, %edi
-; 686-NEXT:    orl %ecx, %edi
-; 686-NEXT:    setne (%esp)
 ; 686-NEXT:    xorl %ecx, %ecx
 ; 686-NEXT:    cmpl $-1, %edx
 ; 686-NEXT:    sete %cl
 ; 686-NEXT:    xorl %edi, %edi
 ; 686-NEXT:    addl $7093, %edx # imm = 0x1BB5
 ; 686-NEXT:    adcxl %edi, %esi
 ; 686-NEXT:    cmpl %ecx, %edx
 ; 686-NEXT:    sbbl $0, %esi