[llvm] r318448 - [SelectionDAG] Consolidate (t|T)ransferDbgValues methods, NFC.

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 16 13:07:51 PST 2017


It looks like this is breaking some bots. It looks like some of the asserts need to be weakened. I'll just revert for now.

vedant

> On Nov 16, 2017, at 11:50 AM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: vedantk
> Date: Thu Nov 16 11:50:24 2017
> New Revision: 318448
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=318448&view=rev
> Log:
> [SelectionDAG] Consolidate (t|T)ransferDbgValues methods, NFC.
> 
> TransferDbgValues (capital 'T') is wired into ReplaceAllUsesWith, and
> transferDbgValues (lowercase 't') is used elsewhere (e.g in Legalize).
> 
> Both functions should be doing the exact same thing. This patch
> consolidates the logic into one place.
> 
> Differential Revision: https://reviews.llvm.org/D40104
> 
> Modified:
>    llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> 
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=318448&r1=318447&r2=318448&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Thu Nov 16 11:50:24 2017
> @@ -1195,9 +1195,10 @@ public:
>                                     unsigned O);
> 
>   /// Transfer debug values from one node to another, while optionally
> -  /// generating fragment expressions for split-up values.
> +  /// generating fragment expressions for split-up values. If \p InvalidateDbg
> +  /// is set, debug values are invalidated after they are transferred.
>   void transferDbgValues(SDValue From, SDValue To, unsigned OffsetInBits = 0,
> -                         unsigned SizeInBits = 0);
> +                         unsigned SizeInBits = 0, bool InvalidateDbg = true);
> 
>   /// Remove the specified node from the system. If any of its
>   /// operands then becomes dead, remove them as well. Inform UpdateListener
> @@ -1279,10 +1280,6 @@ public:
>     return DbgInfo->getSDDbgValues(SD);
>   }
> 
> -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.
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp?rev=318448&r1=318447&r2=318448&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Thu Nov 16 11:50:24 2017
> @@ -845,13 +845,14 @@ void DAGTypeLegalizer::SetExpandedIntege
>   AnalyzeNewValue(Lo);
>   AnalyzeNewValue(Hi);
> 
> -  // Transfer debug values.
> +  // Transfer debug values. Don't invalidate the source debug value until it's
> +  // been transferred to the high and low bits.
>   if (DAG.getDataLayout().isBigEndian()) {
> -    DAG.transferDbgValues(Op, Hi, 0, Hi.getValueSizeInBits());
> +    DAG.transferDbgValues(Op, Hi, 0, Hi.getValueSizeInBits(), false);
>     DAG.transferDbgValues(Op, Lo, Hi.getValueSizeInBits(),
>                           Lo.getValueSizeInBits());
>   } else {
> -    DAG.transferDbgValues(Op, Lo, 0, Lo.getValueSizeInBits());
> +    DAG.transferDbgValues(Op, Lo, 0, Lo.getValueSizeInBits(), false);
>     DAG.transferDbgValues(Op, Hi, Lo.getValueSizeInBits(),
>                           Hi.getValueSizeInBits());
>   }
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=318448&r1=318447&r2=318448&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Thu Nov 16 11:50:24 2017
> @@ -7027,16 +7027,29 @@ SDDbgValue *SelectionDAG::getFrameIndexD
> }
> 
> void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
> -                                     unsigned OffsetInBits,
> -                                     unsigned SizeInBits) {
> +                                     unsigned OffsetInBits, unsigned SizeInBits,
> +                                     bool InvalidateDbg) {
>   SDNode *FromNode = From.getNode();
>   SDNode *ToNode = To.getNode();
> -  assert(FromNode != ToNode);
> +  assert(FromNode && ToNode && "Can't modify dbg values");
> +
> +  // Remove these checks when ReplaceAllUsesWith gets stricter.
> +  // TODO: assert(From != To && "Redundant dbg value transfer");
> +  // TODO: assert(FromNode != ToNode && "Intranode dbg value transfer");
> +  if (From == To || FromNode == ToNode)
> +    return;
> +
> +  if (!FromNode->getHasDebugValue())
> +    return;
> 
>   SmallVector<SDDbgValue *, 2> ClonedDVs;
>   for (SDDbgValue *Dbg : GetDbgValues(FromNode)) {
> -    if (Dbg->getKind() != SDDbgValue::SDNODE)
> -      break;
> +    // Just transfer the dbg value attached to From.
> +    if (Dbg->getResNo() != From.getResNo())
> +      continue;
> +
> +    assert(!Dbg->isInvalidated() && "Invalid dbg value");
> +    assert(Dbg->getKind() == SDDbgValue::SDNODE && "Can't transfer dbg value");
> 
>     DIVariable *Var = Dbg->getVariable();
>     auto *Expr = Dbg->getExpression();
> @@ -7059,7 +7072,9 @@ void SelectionDAG::transferDbgValues(SDV
>         getDbgValue(Var, Expr, ToNode, To.getResNo(), Dbg->isIndirect(),
>                     Dbg->getDebugLoc(), Dbg->getOrder());
>     ClonedDVs.push_back(Clone);
> -    Dbg->setIsInvalidated();
> +
> +    if (InvalidateDbg)
> +      Dbg->setIsInvalidated();
>   }
> 
>   for (SDDbgValue *Dbg : ClonedDVs)
> @@ -7137,7 +7152,7 @@ void SelectionDAG::ReplaceAllUsesWith(SD
>   assert(From != To.getNode() && "Cannot replace uses of with self");
> 
>   // Preserve Debug Values
> -  TransferDbgValues(FromN, To);
> +  transferDbgValues(FromN, To);
> 
>   // Iterate over all the existing uses of From. New uses will be added
>   // to the beginning of the use list, which we avoid visiting.
> @@ -7196,7 +7211,7 @@ void SelectionDAG::ReplaceAllUsesWith(SD
>   for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
>     if (From->hasAnyUseOfValue(i)) {
>       assert((i < To->getNumValues()) && "Invalid To location");
> -      TransferDbgValues(SDValue(From, i), SDValue(To, i));
> +      transferDbgValues(SDValue(From, i), SDValue(To, i));
>     }
> 
>   // Iterate over just the existing users of From. See the comments in
> @@ -7240,7 +7255,7 @@ void SelectionDAG::ReplaceAllUsesWith(SD
> 
>   // Preserve Debug Info.
>   for (unsigned i = 0, e = From->getNumValues(); i != e; ++i)
> -    TransferDbgValues(SDValue(From, i), *To);
> +    transferDbgValues(SDValue(From, i), *To);
> 
>   // Iterate over just the existing users of From. See the comments in
>   // the ReplaceAllUsesWith above.
> @@ -7287,7 +7302,7 @@ void SelectionDAG::ReplaceAllUsesOfValue
>   }
> 
>   // Preserve Debug Info.
> -  TransferDbgValues(From, To);
> +  transferDbgValues(From, To);
> 
>   // Iterate over just the existing users of From. See the comments in
>   // the ReplaceAllUsesWith above.
> @@ -7365,7 +7380,7 @@ void SelectionDAG::ReplaceAllUsesOfValue
>   if (Num == 1)
>     return ReplaceAllUsesOfValueWith(*From, *To);
> 
> -  TransferDbgValues(*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
> @@ -7514,31 +7529,6 @@ void SelectionDAG::AddDbgValue(SDDbgValu
>   DbgInfo->add(DB, SD, isParameter);
> }
> 
> -/// Transfer SDDbgValues. Called in replace nodes.
> -void SelectionDAG::TransferDbgValues(SDValue From, SDValue To) {
> -  if (From == To || !From.getNode()->getHasDebugValue())
> -    return;
> -  SDNode *FromNode = From.getNode();
> -  SDNode *ToNode = To.getNode();
> -  SmallVector<SDDbgValue *, 2> ClonedDVs;
> -  for (auto *Dbg : GetDbgValues(FromNode)) {
> -    // Only add Dbgvalues attached to same ResNo.
> -    if (Dbg->getKind() == SDDbgValue::SDNODE &&
> -        Dbg->getSDNode() == From.getNode() &&
> -        Dbg->getResNo() == From.getResNo() && !Dbg->isInvalidated()) {
> -      assert(FromNode != ToNode &&
> -             "Should not transfer Debug Values intranode");
> -      SDDbgValue *Clone = getDbgValue(Dbg->getVariable(), Dbg->getExpression(),
> -                                      ToNode, To.getResNo(), Dbg->isIndirect(),
> -                                      Dbg->getDebugLoc(), Dbg->getOrder());
> -      ClonedDVs.push_back(Clone);
> -      Dbg->setIsInvalidated();
> -    }
> -  }
> -  for (SDDbgValue *I : ClonedDVs)
> -    AddDbgValue(I, ToNode, false);
> -}
> -
> SDValue SelectionDAG::makeEquivalentMemoryOrdering(LoadSDNode *OldLoad,
>                                                    SDValue NewMemOp) {
>   assert(isa<MemSDNode>(NewMemOp.getNode()) && "Expected a memop node");
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list