[llvm] r273456 - Preserve DebugInfo when replacing values in DAGCombiner

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 17:13:28 PDT 2016


r273518 is the revert.

Peter

On Wed, Jun 22, 2016 at 5:12 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:

> This change caused https://llvm.org/bugs/show_bug.cgi?id=28270 which
> broke our chromium bots. Reverting.
>
> Peter
>
> On Wed, Jun 22, 2016 at 12:03 PM, Nirav Dave via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: niravd
>> Date: Wed Jun 22 14:03:26 2016
>> New Revision: 273456
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=273456&view=rev
>> Log:
>> Preserve DebugInfo when replacing values in DAGCombiner
>>
>> Recommiting after fixing over-aggressive assertion
>>
>> [DAG] Previously debug values would transfer debuginfo for the selected
>> start node for a replacement which allows for debug to be dropped.
>>
>> Push debug value transfer to occur with node/value replacement in
>> SelectionDAG, remove now extraneous transfers of debug values.
>>
>> This refixes PR9817 which was being incompletely checked in the
>> testsuite.
>>
>> Reviewers: jyknight
>>
>> Subscribers: dblaikie, llvm-commits
>>
>> Differential Revision: http://reviews.llvm.org/D21037
>>
>> Modified:
>>     llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>     llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
>>     llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
>>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>>     llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
>>     llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp
>>     llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Wed Jun 22 14:03:26
>> 2016
>> @@ -1221,9 +1221,11 @@ public:
>>      return DbgInfo->getSDDbgValues(SD);
>>    }
>>
>> -  /// Transfer SDDbgValues.
>> +private:
>> +  /// Transfer SDDbgValues. Called via ReplaceAllUses{OfValue}?With
>>    void TransferDbgValues(SDValue From, SDValue To);
>>
>> +public:
>>    /// Return true if there are any SDDbgValue nodes associated
>>    /// with this SelectionDAG.
>>    bool hasDebugValues() const { return !DbgInfo->empty(); }
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -1333,8 +1333,6 @@ void DAGCombiner::Run(CombineLevel AtLev
>>      DEBUG(dbgs() << " ... into: ";
>>            RV.getNode()->dump(&DAG));
>>
>> -    // Transfer debug value.
>> -    DAG.TransferDbgValues(SDValue(N, 0), RV);
>>      if (N->getNumValues() == RV.getNode()->getNumValues())
>>        DAG.ReplaceAllUsesWith(N, RV.getNode());
>>      else {
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -320,7 +320,6 @@ InstrEmitter::AddRegisterOperand(Machine
>>           "Chain and glue operands should occur at end of operand list!");
>>    // Get/emit the operand.
>>    unsigned VReg = getVR(Op, VRBaseMap);
>> -  assert(TargetRegisterInfo::isVirtualRegister(VReg) && "Not a vreg?");
>>
>>    const MCInstrDesc &MCID = MIB->getDesc();
>>    bool isOptDef = IIOpNum < MCID.getNumOperands() &&
>> @@ -334,6 +333,8 @@ InstrEmitter::AddRegisterOperand(Machine
>>      const TargetRegisterClass *DstRC = nullptr;
>>      if (IIOpNum < II->getNumOperands())
>>        DstRC =
>> TRI->getAllocatableClass(TII->getRegClass(*II,IIOpNum,TRI,*MF));
>> +    assert((!DstRC || TargetRegisterInfo::isVirtualRegister(VReg)) &&
>> +           "Expected VReg");
>>      if (DstRC && !MRI->constrainRegClass(VReg, DstRC, MinRCSize)) {
>>        unsigned NewVReg = MRI->createVirtualRegister(DstRC);
>>        BuildMI(*MBB, InsertPos, Op.getNode()->getDebugLoc(),
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -179,8 +179,6 @@ public:
>>             "Replacing one node with another that produces a different
>> number "
>>             "of values!");
>>      DAG.ReplaceAllUsesWith(Old, New);
>> -    for (unsigned i = 0, e = Old->getNumValues(); i != e; ++i)
>> -      DAG.TransferDbgValues(SDValue(Old, i), SDValue(New, i));
>>      if (UpdatedNodes)
>>        UpdatedNodes->insert(New);
>>      ReplacedNode(Old);
>> @@ -190,7 +188,6 @@ public:
>>            dbgs() << "     with:      "; New->dump(&DAG));
>>
>>      DAG.ReplaceAllUsesWith(Old, New);
>> -    DAG.TransferDbgValues(Old, New);
>>      if (UpdatedNodes)
>>        UpdatedNodes->insert(New.getNode());
>>      ReplacedNode(Old.getNode());
>> @@ -203,7 +200,6 @@ public:
>>        DEBUG(dbgs() << (i == 0 ? "     with:      "
>>                                : "      and:      ");
>>              New[i]->dump(&DAG));
>> -      DAG.TransferDbgValues(SDValue(Old, i), New[i]);
>>        if (UpdatedNodes)
>>          UpdatedNodes->insert(New[i].getNode());
>>      }
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -6333,6 +6333,9 @@ void SelectionDAG::ReplaceAllUsesWith(SD
>>      AddModifiedNodeToCSEMaps(User);
>>    }
>>
>> +  // Preserve Debug Values
>> +  TransferDbgValues(FromN, To);
>> +
>>    // If we just RAUW'd the root, take note.
>>    if (FromN == getRoot())
>>      setRoot(To);
>> @@ -6356,6 +6359,11 @@ void SelectionDAG::ReplaceAllUsesWith(SD
>>    if (From == To)
>>      return;
>>
>> +  // Preserve Debug Info. Only do this if there's a use.
>> +  for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
>> +    if (From->hasAnyUseOfValue(i))
>> +      TransferDbgValues(SDValue(From, i), SDValue(To, i));
>> +
>>    // Iterate over just the existing users of From. See the comments in
>>    // the ReplaceAllUsesWith above.
>>    SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
>> @@ -6395,6 +6403,10 @@ void SelectionDAG::ReplaceAllUsesWith(SD
>>    if (From->getNumValues() == 1)  // Handle the simple case efficiently.
>>      return ReplaceAllUsesWith(SDValue(From, 0), To[0]);
>>
>> +  // Preserve Debug Info.
>> +  for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
>> +    TransferDbgValues(SDValue(From, i), *To);
>> +
>>    // Iterate over just the existing users of From. See the comments in
>>    // the ReplaceAllUsesWith above.
>>    SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
>> @@ -6439,6 +6451,9 @@ void SelectionDAG::ReplaceAllUsesOfValue
>>      return;
>>    }
>>
>> +  // Preserve Debug Info.
>> +  TransferDbgValues(From, To);
>> +
>>    // Iterate over just the existing users of From. See the comments in
>>    // the ReplaceAllUsesWith above.
>>    SDNode::use_iterator UI = From.getNode()->use_begin(),
>> @@ -6513,6 +6528,8 @@ void SelectionDAG::ReplaceAllUsesOfValue
>>    if (Num == 1)
>>      return ReplaceAllUsesOfValueWith(*From, *To);
>>
>> +  TransferDbgValues(*From, *To);
>> +
>>    // Read up all the uses and make records of them. This helps
>>    // processing new uses that are introduced during the
>>    // replacement process.
>> @@ -6661,7 +6678,7 @@ void SelectionDAG::AddDbgValue(SDDbgValu
>>    DbgInfo->add(DB, SD, isParameter);
>>  }
>>
>> -/// TransferDbgValues - Transfer SDDbgValues.
>> +/// TransferDbgValues - Transfer SDDbgValues. Called in replace nodes.
>>  void SelectionDAG::TransferDbgValues(SDValue From, SDValue To) {
>>    if (From == To || !From.getNode()->getHasDebugValue())
>>      return;
>>
>> Modified: llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp (original)
>> +++ llvm/trunk/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -1308,8 +1308,6 @@ void HexagonDAGToDAGISel::SelectFrameInd
>>      R = CurDAG->getMachineNode(Hexagon::TFR_FIA, DL, MVT::i32, Ops);
>>    }
>>
>> -  if (N->getHasDebugValue())
>> -    CurDAG->TransferDbgValues(SDValue(N, 0), SDValue(R, 0));
>>    ReplaceNode(N, R);
>>  }
>>
>>
>> Modified: llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/Hexagon/HexagonISelLowering.cpp Wed Jun 22
>> 14:03:26 2016
>> @@ -1057,8 +1057,8 @@ HexagonTargetLowering::LowerDYNAMIC_STAC
>>    SDValue AC = DAG.getConstant(A, dl, MVT::i32);
>>    SDVTList VTs = DAG.getVTList(MVT::i32, MVT::Other);
>>    SDValue AA = DAG.getNode(HexagonISD::ALLOCA, dl, VTs, Chain, Size, AC);
>> -  if (Op.getNode()->getHasDebugValue())
>> -    DAG.TransferDbgValues(Op, AA);
>> +
>> +  DAG.ReplaceAllUsesOfValueWith(Op, AA);
>>    return AA;
>>  }
>>
>>
>> Modified: llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll?rev=273456&r1=273455&r2=273456&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll (original)
>> +++ llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll Wed Jun 22
>> 14:03:26 2016
>> @@ -3,6 +3,13 @@ target datalayout = "e-p:32:32:32-i1:8:8
>>  target triple = "i686-apple-darwin"
>>  ; PR 9817
>>
>> +; There should be a DEBUG_VALUE for each call to llvm.dbg.value
>> +
>> +; CHECK:  ##DEBUG_VALUE: __OpenCL_test_kernel:ip <-
>> +; CHECK:  ##DEBUG_VALUE: xxx <- 0
>> +; CHECK:  ##DEBUG_VALUE: gid <- %E{{..$}}
>> +; CHECK:  ##DEBUG_VALUE: idx <- %E{{..$}}
>> +; CHECK-NOT:  ##DEBUG_VALUE:
>>
>>  declare <4 x i32> @__amdil_get_global_id_int()
>>  declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
>> @@ -12,10 +19,9 @@ entry:
>>    %0 = call <4 x i32> @__amdil_get_global_id_int() nounwind
>>    %1 = extractelement <4 x i32> %0, i32 0
>>    call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !9,
>> metadata !DIExpression()), !dbg !11
>> -  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !13,
>> metadata !DIExpression()), !dbg !14
>> +  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !21,
>> metadata !DIExpression()), !dbg !14
>>    %tmp2 = load i32, i32 addrspace(1)* %ip, align 4, !dbg !15
>>    %tmp3 = add i32 0, %tmp2, !dbg !15
>> -; CHECK:  ##DEBUG_VALUE: idx <- %E{{..$}}
>>    call void @llvm.dbg.value(metadata i32 %tmp3, i64 0, metadata !13,
>> metadata !DIExpression()), !dbg !15
>>    %arrayidx = getelementptr i32, i32 addrspace(1)* %ip, i32 %1, !dbg !16
>>    store i32 %tmp3, i32 addrspace(1)* %arrayidx, align 4, !dbg !16
>> @@ -44,3 +50,4 @@ entry:
>>  !17 = !DILocation(line: 7, column: 1, scope: !0)
>>  !19 = !DIFile(filename: "OCL6368.tmp.cl", directory:
>> "E:\5CUsers\5Cmvillmow.AMD\5CAppData\5CLocal\5CTemp")
>>  !20 = !{i32 1, !"Debug Info Version", i32 3}
>> +!21 = !DILocalVariable(name: "xxx", line: 4, scope: !10, file: !1, type:
>> !6)
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> --
> Peter
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/e7fd95b5/attachment.html>


More information about the llvm-commits mailing list