[llvm-commits] [llvm] r99469 - in /llvm/trunk: include/llvm/CodeGen/MachineOperand.h include/llvm/CodeGen/SelectionDAG.h lib/CodeGen/SelectionDAG/InstrEmitter.cpp lib/CodeGen/SelectionDAG/InstrEmitter.h lib/CodeGen/SelectionDAG/SDNodeDbgValue.h l

Jeffrey Yasskin jyasskin at google.com
Fri Mar 26 22:16:51 PDT 2010


Hey Evan, this change may have introduced the new memory leak in
DebugInfo/2010-02-01-DbgValueCrash.ll
(http://google1.osuosl.org:8011/builders/llvm-x86_64-linux-vg_leak/builds/7/steps/test-llvm/logs/stdio).
The whole valgrind error (run at r99590) is:

==8278== 56 bytes in 1 blocks are definitely lost in loss record 130 of 135
==8278==    at 0x4C229C7: operator new(unsigned long) (vg_replace_malloc.c:220)
==8278==    by 0xD809F6:
llvm::SelectionDAGBuilder::visitIntrinsicCall(llvm::CallInst&,
unsigned int) (SelectionDAGBuilder.cpp:3840)
==8278==    by 0xD87327:
llvm::SelectionDAGBuilder::visitCall(llvm::CallInst&)
(SelectionDAGBuilder.cpp:4662)
==8278==    by 0xD62E31: llvm::SelectionDAGBuilder::visit(unsigned
int, llvm::User&) (Instruction.def:161)
==8278==    by 0xD6293D:
llvm::SelectionDAGBuilder::visit(llvm::Instruction&)
(SelectionDAGBuilder.cpp:617)
==8278==    by 0xDA455C:
llvm::SelectionDAGISel::SelectBasicBlock(llvm::BasicBlock*,
llvm::ilist_iterator<llvm::Instruction>,
llvm::ilist_iterator<llvm::Instruction>, bool&)
(SelectionDAGISel.cpp:407)
==8278==    by 0xDA7952:
llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function&,
llvm::MachineFunction&, llvm::MachineModuleInfo*, llvm::DwarfWriter*,
llvm::TargetInstrInfo const&) (SelectionDAGISel.cpp:1030)
==8278==    by 0xDA41E6:
llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
(SelectionDAGISel.cpp:344)
==8278==    by 0xEBE6B6:
llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
(MachineFunctionPass.cpp:27)
==8278==    by 0x11E2A5C:
llvm::FPPassManager::runOnFunction(llvm::Function&)
(PassManager.cpp:1350)
==8278==    by 0x11E2734:
llvm::FunctionPassManagerImpl::run(llvm::Function&)
(PassManager.cpp:1301)
==8278==    by 0x11E23E4:
llvm::FunctionPassManager::run(llvm::Function&) (PassManager.cpp:1231)
==8278==

Could you take a look? Sorry if you've already fixed this.

On Wed, Mar 24, 2010 at 6:38 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> Author: evancheng
> Date: Wed Mar 24 20:38:16 2010
> New Revision: 99469
>
> URL: http://llvm.org/viewvc/llvm-project?rev=99469&view=rev
> Log:
> Change how dbg_value sdnodes are converted into machine instructions. Their placement should be determined by the relative order of incoming llvm instructions. The scheduler will now use the SDNode ordering information to determine where to insert them. A dbg_value instruction is inserted after the instruction with the last highest source order and before the instruction with the next highest source order. It will optimize the placement by inserting right after the instruction that produces the value if they have consecutive order numbers.
>
> Here is a theoretical example that illustrates why the placement is important.
>
> tmp1 =
> store tmp1 -> x
> ...
> tmp2 = add ...
> ...
> call
> ...
> store tmp2 -> x
>
> Now mem2reg comes along:
>
> tmp1 =
> dbg_value (tmp1 -> x)
> ...
> tmp2 = add ...
> ...
> call
> ...
> dbg_value (tmp2 -> x)
>
> When the debugger examine the value of x after the add instruction but before the call, it should have the value of tmp1.
>
> Furthermore, for dbg_value's that reference constants, they should not be emitted at the beginning of the block (since they do not have "producers").
>
> This patch also cleans up how SDISel manages DbgValue nodes. It allow a SDNode to be referenced by multiple SDDbgValue nodes. When a SDNode is deleted, it uses the information to find the SDDbgValues and invalidate them. They are not deleted until the corresponding SelectionDAG is destroyed.
>
> Modified:
>    llvm/trunk/include/llvm/CodeGen/MachineOperand.h
>    llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineOperand.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineOperand.h?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineOperand.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineOperand.h Wed Mar 24 20:38:16 2010
> @@ -285,6 +285,11 @@
>     IsEarlyClobber = Val;
>   }
>
> +  void setIsDebug(bool Val = true) {
> +    assert(isReg() && IsDef && "Wrong MachineOperand accessor");
> +    IsDebug = Val;
> +  }
> +
>   //===--------------------------------------------------------------------===//
>   // Accessors for various operand types.
>   //===--------------------------------------------------------------------===//
>
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Wed Mar 24 20:38:16 2010
> @@ -60,42 +60,40 @@
>
>  /// SDDbgInfo - Keeps track of dbg_value information through SDISel.  We do
>  /// not build SDNodes for these so as not to perturb the generated code;
> -/// instead the info is kept off to the side in this structure.  SDNodes may
> -/// have an associated dbg_value entry in DbgValMap.  Debug info that is not
> -/// associated with any SDNode is held in DbgConstMap.  It is possible for
> -/// optimizations to change a variable to a constant, in which case the
> -/// corresponding debug info is moved from the variable to the constant table
> -/// (NYI).
> +/// instead the info is kept off to the side in this structure. Each SDNode may
> +/// have one or more associated dbg_value entries. This information is kept in
> +/// DbgValMap.
>  class SDDbgInfo {
> -  DenseMap<const SDNode*, SDDbgValue*> DbgVblMap;
> -  SmallVector<SDDbgValue*, 4> DbgConstMap;
> +  SmallVector<SDDbgValue*, 32> DbgValues;
> +  DenseMap<const SDNode*, SmallVector<SDDbgValue*, 2> > DbgVblMap;
>
>   void operator=(const SDDbgInfo&);   // Do not implement.
>   SDDbgInfo(const SDDbgInfo&);   // Do not implement.
>  public:
>   SDDbgInfo() {}
>
> -  void add(const SDNode *Node, SDDbgValue *V) {
> -    DbgVblMap[Node] = V;
> +  void add(SDDbgValue *V, const SDNode *Node = 0) {
> +    if (Node)
> +      DbgVblMap[Node].push_back(V);
> +    DbgValues.push_back(V);
>   }
> -  void add(SDDbgValue *V) { DbgConstMap.push_back(V); }
> -  void remove(const SDNode *Node) {
> -    DenseMap<const SDNode*, SDDbgValue*>::iterator Itr =
> -                      DbgVblMap.find(Node);
> -    if (Itr != DbgVblMap.end())
> -      DbgVblMap.erase(Itr);
> -  }
> -  // No need to remove a constant.
> +
>   void clear() {
>     DbgVblMap.clear();
> -    DbgConstMap.clear();
> +    DbgValues.clear();
> +  }
> +
> +  bool empty() const {
> +    return DbgValues.empty();
>   }
> -  SDDbgValue *getSDDbgValue(const SDNode *Node) {
> +
> +  SmallVector<SDDbgValue*,2> &getSDDbgValues(const SDNode *Node) {
>     return DbgVblMap[Node];
>   }
> -  typedef SmallVector<SDDbgValue*, 4>::iterator ConstDbgIterator;
> -  ConstDbgIterator DbgConstBegin() { return DbgConstMap.begin(); }
> -  ConstDbgIterator DbgConstEnd() { return DbgConstMap.end(); }
> +
> +  typedef SmallVector<SDDbgValue*,32>::iterator DbgIterator;
> +  DbgIterator DbgBegin() { return DbgValues.begin(); }
> +  DbgIterator DbgEnd()   { return DbgValues.end(); }
>  };
>
>  enum CombineLevel {
> @@ -871,19 +869,21 @@
>   /// GetOrdering - Get the order for the SDNode.
>   unsigned GetOrdering(const SDNode *SD) const;
>
> -  /// AssignDbgInfo - Assign debug info to the SDNode.
> -  void AssignDbgInfo(SDNode *SD, SDDbgValue *db);
> +  /// AddDbgValue - Add a dbg_value SDNode. If SD is non-null that means the
> +  /// value is produced by SD.
> +  void AddDbgValue(SDDbgValue *DB, SDNode *SD = 0);
>
> -  /// RememberDbgInfo - Remember debug info with no associated SDNode.
> -  void RememberDbgInfo(SDDbgValue *db);
> +  /// GetDbgValues - Get the debug values which reference the given SDNode.
> +  SmallVector<SDDbgValue*,2> &GetDbgValues(const SDNode* SD) {
> +    return DbgInfo->getSDDbgValues(SD);
> +  }
>
> -  /// GetDbgInfo - Get the debug info for the SDNode.
> -  SDDbgValue *GetDbgInfo(const SDNode* SD);
> +  /// hasDebugValues - Return true if there are any SDDbgValue nodes associated
> +  /// with this SelectionDAG.
> +  bool hasDebugValues() const { return !DbgInfo->empty(); }
>
> -  SDDbgInfo::ConstDbgIterator DbgConstBegin() {
> -    return DbgInfo->DbgConstBegin();
> -  }
> -  SDDbgInfo::ConstDbgIterator DbgConstEnd() { return DbgInfo->DbgConstEnd(); }
> +  SDDbgInfo::DbgIterator DbgBegin() { return DbgInfo->DbgBegin(); }
> +  SDDbgInfo::DbgIterator DbgEnd()   { return DbgInfo->DbgEnd(); }
>
>   void dump() const;
>
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp Wed Mar 24 20:38:16 2010
> @@ -264,7 +264,8 @@
>  InstrEmitter::AddRegisterOperand(MachineInstr *MI, SDValue Op,
>                                  unsigned IIOpNum,
>                                  const TargetInstrDesc *II,
> -                                 DenseMap<SDValue, unsigned> &VRBaseMap) {
> +                                 DenseMap<SDValue, unsigned> &VRBaseMap,
> +                                 bool IsDebug) {
>   assert(Op.getValueType() != MVT::Other &&
>          Op.getValueType() != MVT::Flag &&
>          "Chain and flag operands should occur at end of operand list!");
> @@ -295,7 +296,11 @@
>     }
>   }
>
> -  MI->addOperand(MachineOperand::CreateReg(VReg, isOptDef));
> +  MI->addOperand(MachineOperand::CreateReg(VReg, isOptDef,
> +                                           false/*isImp*/, false/*isKill*/,
> +                                           false/*isDead*/, false/*isUndef*/,
> +                                           false/*isEarlyClobber*/,
> +                                           0/*SubReg*/, IsDebug));
>  }
>
>  /// AddOperand - Add the specified operand to the specified machine instr.  II
> @@ -305,9 +310,10 @@
>  void InstrEmitter::AddOperand(MachineInstr *MI, SDValue Op,
>                               unsigned IIOpNum,
>                               const TargetInstrDesc *II,
> -                              DenseMap<SDValue, unsigned> &VRBaseMap) {
> +                              DenseMap<SDValue, unsigned> &VRBaseMap,
> +                              bool IsDebug) {
>   if (Op.isMachineOpcode()) {
> -    AddRegisterOperand(MI, Op, IIOpNum, II, VRBaseMap);
> +    AddRegisterOperand(MI, Op, IIOpNum, II, VRBaseMap, IsDebug);
>   } else if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op)) {
>     MI->addOperand(MachineOperand::CreateImm(C->getSExtValue()));
>   } else if (ConstantFPSDNode *F = dyn_cast<ConstantFPSDNode>(Op)) {
> @@ -356,7 +362,7 @@
>     assert(Op.getValueType() != MVT::Other &&
>            Op.getValueType() != MVT::Flag &&
>            "Chain and flag operands should occur at end of operand list!");
> -    AddRegisterOperand(MI, Op, IIOpNum, II, VRBaseMap);
> +    AddRegisterOperand(MI, Op, IIOpNum, II, VRBaseMap, IsDebug);
>   }
>  }
>
> @@ -498,75 +504,48 @@
>   assert(isNew && "Node emitted out of order - early");
>  }
>
> -/// EmitDbgValue - Generate any debug info that refers to this Node.  Constant
> -/// dbg_value is not handled here.
> -void
> -InstrEmitter::EmitDbgValue(SDNode *Node,
> -                           DenseMap<SDValue, unsigned> &VRBaseMap,
> -                           SDDbgValue *sd) {
> -  if (!Node->getHasDebugValue())
> -    return;
> -  if (!sd)
> -    return;
> -  assert(sd->getKind() == SDDbgValue::SDNODE);
> -  unsigned VReg = getVR(SDValue(sd->getSDNode(), sd->getResNo()), VRBaseMap);
> -  const TargetInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
> -  DebugLoc DL = sd->getDebugLoc();
> -  MachineInstr *MI;
> -  if (VReg) {
> -    MI = BuildMI(*MF, DL, II).addReg(VReg, RegState::Debug).
> -                              addImm(sd->getOffset()).
> -                              addMetadata(sd->getMDPtr());
> -  } else {
> -    // Insert an Undef so we can see what we dropped.
> -    MI = BuildMI(*MF, DL, II).addReg(0U).addImm(sd->getOffset()).
> -                                    addMetadata(sd->getMDPtr());
> -  }
> -  MBB->insert(InsertPos, MI);
> -}
> -
> -/// EmitDbgValue - Generate debug info that does not refer to a SDNode.
> -void
> -InstrEmitter::EmitDbgValue(SDDbgValue *sd,
> +/// EmitDbgValue - Generate machine instruction for a dbg_value node.
> +///
> +MachineInstr *InstrEmitter::EmitDbgValue(SDDbgValue *SD,
> +                                         MachineBasicBlock *InsertBB,
> +                                         DenseMap<SDValue, unsigned> &VRBaseMap,
>                          DenseMap<MachineBasicBlock*, MachineBasicBlock*> *EM) {
> -  if (!sd)
> -    return;
> +  uint64_t Offset = SD->getOffset();
> +  MDNode* MDPtr = SD->getMDPtr();
> +  DebugLoc DL = SD->getDebugLoc();
> +
>   const TargetInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
> -  uint64_t Offset = sd->getOffset();
> -  MDNode* mdPtr = sd->getMDPtr();
> -  SDDbgValue::DbgValueKind kind = sd->getKind();
> -  DebugLoc DL = sd->getDebugLoc();
> -  MachineInstr* MI;
> -  if (kind == SDDbgValue::CONST) {
> -    Value *V = sd->getConst();
> +  MachineInstrBuilder MIB = BuildMI(*MF, DL, II);
> +  if (SD->getKind() == SDDbgValue::SDNODE) {
> +    AddOperand(&*MIB, SDValue(SD->getSDNode(), SD->getResNo()),
> +               (*MIB).getNumOperands(), &II, VRBaseMap, true /*IsDebug*/);
> +  } else if (SD->getKind() == SDDbgValue::CONST) {
> +    Value *V = SD->getConst();
>     if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
> -      MI = BuildMI(*MF, DL, II).addImm(CI->getZExtValue()).
> -                                     addImm(Offset).addMetadata(mdPtr);
> +      MIB.addImm(CI->getSExtValue());
>     } else if (ConstantFP *CF = dyn_cast<ConstantFP>(V)) {
> -      MI = BuildMI(*MF, DL, II).addFPImm(CF).
> -                                     addImm(Offset).addMetadata(mdPtr);
> +      MIB.addFPImm(CF);
>     } else {
>       // Could be an Undef.  In any case insert an Undef so we can see what we
>       // dropped.
> -      MI = BuildMI(*MF, DL, II).addReg(0U).
> -                                       addImm(Offset).addMetadata(mdPtr);
> +      MIB.addReg(0U);
>     }
> -  } else if (kind == SDDbgValue::FRAMEIX) {
> -    unsigned FrameIx = sd->getFrameIx();
> +  } else if (SD->getKind() == SDDbgValue::FRAMEIX) {
> +    unsigned FrameIx = SD->getFrameIx();
>     // Stack address; this needs to be lowered in target-dependent fashion.
>     // FIXME test that the target supports this somehow; if not emit Undef.
>     // Create a pseudo for EmitInstrWithCustomInserter's consumption.
> -    MI = BuildMI(*MF, DL, II).addImm(FrameIx).
> -                                   addImm(Offset).addMetadata(mdPtr);
> -    MBB = TLI->EmitInstrWithCustomInserter(MI, MBB, EM);
> -    InsertPos = MBB->end();
> -    return;
> +    MIB.addImm(FrameIx).addImm(Offset).addMetadata(MDPtr);
> +    abort();
> +    TLI->EmitInstrWithCustomInserter(&*MIB, InsertBB, EM);
> +    return 0;
>   } else {
>     // Insert an Undef so we can see what we dropped.
> -    MI = BuildMI(*MF, DL, II).addReg(0U).
> -                                     addImm(Offset).addMetadata(mdPtr);
> +    MIB.addReg(0U);
>   }
> -  MBB->insert(InsertPos, MI);
> +
> +  MIB.addImm(Offset).addMetadata(MDPtr);
> +  return &*MIB;
>  }
>
>  /// EmitNode - Generate machine code for a node and needed dependencies.
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.h?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.h (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.h Wed Mar 24 20:38:16 2010
> @@ -64,7 +64,8 @@
>   void AddRegisterOperand(MachineInstr *MI, SDValue Op,
>                           unsigned IIOpNum,
>                           const TargetInstrDesc *II,
> -                          DenseMap<SDValue, unsigned> &VRBaseMap);
> +                          DenseMap<SDValue, unsigned> &VRBaseMap,
> +                          bool IsDebug = false);
>
>   /// AddOperand - Add the specified operand to the specified machine instr.  II
>   /// specifies the instruction information for the node, and IIOpNum is the
> @@ -73,7 +74,8 @@
>   void AddOperand(MachineInstr *MI, SDValue Op,
>                   unsigned IIOpNum,
>                   const TargetInstrDesc *II,
> -                  DenseMap<SDValue, unsigned> &VRBaseMap);
> +                  DenseMap<SDValue, unsigned> &VRBaseMap,
> +                  bool IsDebug = false);
>
>   /// EmitSubregNode - Generate machine code for subreg nodes.
>   ///
> @@ -98,16 +100,12 @@
>   /// MachineInstr.
>   static unsigned CountOperands(SDNode *Node);
>
> -  /// EmitDbgValue - Generate any debug info that refers to this Node.  Constant
> -  /// dbg_value is not handled here.
> -  void EmitDbgValue(SDNode *Node,
> -                    DenseMap<SDValue, unsigned> &VRBaseMap,
> -                    SDDbgValue* sd);
> -
> -
> -  /// EmitDbgValue - Generate a constant DBG_VALUE.  No node is involved.
> -  void EmitDbgValue(SDDbgValue* sd,
> -                DenseMap<MachineBasicBlock*, MachineBasicBlock*> *EM);
> +  /// EmitDbgValue - Generate machine instruction for a dbg_value node.
> +  ///
> +  MachineInstr *EmitDbgValue(SDDbgValue *SD,
> +                          MachineBasicBlock *InsertBB,
> +                          DenseMap<SDValue, unsigned> &VRBaseMap,
> +                          DenseMap<MachineBasicBlock*, MachineBasicBlock*> *EM);
>
>   /// EmitNode - Generate machine code for a node and needed dependencies.
>   ///
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h Wed Mar 24 20:38:16 2010
> @@ -47,10 +47,12 @@
>   uint64_t Offset;
>   DebugLoc DL;
>   unsigned Order;
> +  bool Invalid;
>  public:
>   // Constructor for non-constants.
>   SDDbgValue(MDNode *mdP, SDNode *N, unsigned R, uint64_t off, DebugLoc dl,
> -             unsigned O) : mdPtr(mdP), Offset(off), DL(dl), Order(O) {
> +             unsigned O) : mdPtr(mdP), Offset(off), DL(dl), Order(O),
> +                           Invalid(false) {
>     kind = SDNODE;
>     u.s.Node = N;
>     u.s.ResNo = R;
> @@ -97,6 +99,12 @@
>   // Returns the SDNodeOrder.  This is the order of the preceding node in the
>   // input.
>   unsigned getOrder() { return Order; }
> +
> +  // setIsInvalidated / isInvalidated - Setter / getter of the "Invalidated"
> +  // property. A SDDbgValue is invalid if the SDNode that produces the value is
> +  // deleted.
> +  void setIsInvalidated() { Invalid = true; }
> +  bool isInvalidated() { return Invalid; }
>  };
>
>  } // end llvm namespace
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp Wed Mar 24 20:38:16 2010
> @@ -23,6 +23,7 @@
>  #include "llvm/Target/TargetSubtarget.h"
>  #include "llvm/ADT/DenseMap.h"
>  #include "llvm/ADT/SmallPtrSet.h"
> +#include "llvm/ADT/SmallSet.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/ADT/Statistic.h"
>  #include "llvm/Support/Debug.h"
> @@ -407,19 +408,67 @@
>   }
>  }
>
> +namespace {
> +  struct OrderSorter {
> +    bool operator()(const std::pair<unsigned, MachineInstr*> &A,
> +                    const std::pair<unsigned, MachineInstr*> &B) {
> +      return A.first < B.first;
> +    }
> +  };
> +}
> +
> +// ProcessSourceNode - Process nodes with source order numbers. These are added
> +// to a vector which EmitSchedule use to determine how to insert dbg_value
> +// instructions in the right order.
> +static void ProcessSourceNode(SDNode *N, SelectionDAG *DAG,
> +                           InstrEmitter &Emitter,
> +                           DenseMap<MachineBasicBlock*, MachineBasicBlock*> *EM,
> +                           DenseMap<SDValue, unsigned> &VRBaseMap,
> +                    SmallVector<std::pair<unsigned, MachineInstr*>, 32> &Orders,
> +                           SmallSet<unsigned, 8> &Seen) {
> +  unsigned Order = DAG->GetOrdering(N);
> +  if (!Order || !Seen.insert(Order))
> +    return;
> +
> +  MachineBasicBlock *BB = Emitter.getBlock();
> +  if (BB->empty() || BB->back().isPHI()) {
> +    // Did not insert any instruction.
> +    Orders.push_back(std::make_pair(Order, (MachineInstr*)0));
> +    return;
> +  }
> +
> +  Orders.push_back(std::make_pair(Order, &BB->back()));
> +  if (!N->getHasDebugValue())
> +    return;
> +  // Opportunistically insert immediate dbg_value uses, i.e. those with source
> +  // order number right after the N.
> +  MachineBasicBlock::iterator InsertPos = Emitter.getInsertPos();
> +  SmallVector<SDDbgValue*,2> &DVs = DAG->GetDbgValues(N);
> +  for (unsigned i = 0, e = DVs.size(); i != e; ++i) {
> +    if (DVs[i]->isInvalidated())
> +      continue;
> +    unsigned DVOrder = DVs[i]->getOrder();
> +    if (DVOrder == ++Order) {
> +      // FIXME: If the source node with next higher order is scheduled before
> +      // this could end up generating funky debug info.
> +      MachineInstr *DbgMI = Emitter.EmitDbgValue(DVs[i], BB, VRBaseMap, EM);
> +      Orders.push_back(std::make_pair(DVOrder, DbgMI));
> +      BB->insert(InsertPos, DbgMI);
> +      DVs[i]->setIsInvalidated();
> +    }
> +  }
> +}
> +
> +
>  /// EmitSchedule - Emit the machine code in scheduled order.
>  MachineBasicBlock *ScheduleDAGSDNodes::
>  EmitSchedule(DenseMap<MachineBasicBlock*, MachineBasicBlock*> *EM) {
>   InstrEmitter Emitter(BB, InsertPos);
>   DenseMap<SDValue, unsigned> VRBaseMap;
>   DenseMap<SUnit*, unsigned> CopyVRBaseMap;
> -
> -  // For now, any constant debug info nodes go at the beginning.
> -  for (SDDbgInfo::ConstDbgIterator I = DAG->DbgConstBegin(),
> -       E = DAG->DbgConstEnd(); I!=E; I++) {
> -    Emitter.EmitDbgValue(*I, EM);
> -    delete *I;
> -  }
> +  SmallVector<std::pair<unsigned, MachineInstr*>, 32> Orders;
> +  SmallSet<unsigned, 8> Seen;
> +  bool HasDbg = DAG->hasDebugValues();
>
>   for (unsigned i = 0, e = Sequence.size(); i != e; i++) {
>     SUnit *SU = Sequence[i];
> @@ -442,22 +491,72 @@
>          N = N->getFlaggedNode())
>       FlaggedNodes.push_back(N);
>     while (!FlaggedNodes.empty()) {
> +      SDNode *N = FlaggedNodes.back();
>       Emitter.EmitNode(FlaggedNodes.back(), SU->OrigNode != SU, SU->isCloned,
>                        VRBaseMap, EM);
> -      if (FlaggedNodes.back()->getHasDebugValue())
> -        if (SDDbgValue *sd = DAG->GetDbgInfo(FlaggedNodes.back())) {
> -          Emitter.EmitDbgValue(FlaggedNodes.back(), VRBaseMap, sd);
> -          delete sd;
> -        }
> +      // Remember the the source order of the inserted instruction.
> +      if (HasDbg)
> +        ProcessSourceNode(N, DAG, Emitter, EM, VRBaseMap, Orders, Seen);
>       FlaggedNodes.pop_back();
>     }
>     Emitter.EmitNode(SU->getNode(), SU->OrigNode != SU, SU->isCloned,
>                      VRBaseMap, EM);
> -    if (SU->getNode()->getHasDebugValue())
> -      if (SDDbgValue *sd = DAG->GetDbgInfo(SU->getNode())) {
> -        Emitter.EmitDbgValue(SU->getNode(), VRBaseMap, sd);
> -        delete sd;
> +    // Remember the the source order of the inserted instruction.
> +    if (HasDbg)
> +      ProcessSourceNode(SU->getNode(), DAG, Emitter, EM, VRBaseMap, Orders,
> +                        Seen);
> +  }
> +
> +  // Insert all the dbg_value which have not already been inserted in source
> +  // order sequence.
> +  if (HasDbg) {
> +    MachineBasicBlock::iterator BBBegin = BB->empty() ? BB->end() : BB->begin();
> +    while (BBBegin != BB->end() && BBBegin->isPHI())
> +      ++BBBegin;
> +
> +    // Sort the source order instructions and use the order to insert debug
> +    // values.
> +    std::sort(Orders.begin(), Orders.end(), OrderSorter());
> +
> +    SDDbgInfo::DbgIterator DI = DAG->DbgBegin();
> +    SDDbgInfo::DbgIterator DE = DAG->DbgEnd();
> +    // Now emit the rest according to source order.
> +    unsigned LastOrder = 0;
> +    MachineInstr *LastMI = 0;
> +    for (unsigned i = 0, e = Orders.size(); i != e && DI != DE; ++i) {
> +      unsigned Order = Orders[i].first;
> +      MachineInstr *MI = Orders[i].second;
> +      // Insert all SDDbgValue's whose order(s) are before "Order".
> +      if (!MI)
> +        continue;
> +      MachineBasicBlock *MIBB = MI->getParent();
> +      for (; DI != DE &&
> +             (*DI)->getOrder() >= LastOrder && (*DI)->getOrder() < Order; ++DI) {
> +        if ((*DI)->isInvalidated())
> +          continue;
> +        MachineInstr *DbgMI = Emitter.EmitDbgValue(*DI, MIBB, VRBaseMap, EM);
> +        if (!LastOrder)
> +          // Insert to start of the BB (after PHIs).
> +          BB->insert(BBBegin, DbgMI);
> +        else {
> +          MachineBasicBlock::iterator Pos = MI;
> +          MIBB->insert(llvm::next(Pos), DbgMI);
> +        }
>       }
> +      LastOrder = Order;
> +      LastMI = MI;
> +    }
> +    // Add trailing DbgValue's before the terminator. FIXME: May want to add
> +    // some of them before one or more conditional branches?
> +    while (DI != DE) {
> +      MachineBasicBlock *InsertBB = Emitter.getBlock();
> +      MachineBasicBlock::iterator Pos= Emitter.getBlock()->getFirstTerminator();
> +      if (!(*DI)->isInvalidated()) {
> +        MachineInstr *DbgMI= Emitter.EmitDbgValue(*DI, InsertBB, VRBaseMap, EM);
> +        InsertBB->insert(Pos, DbgMI);
> +      }
> +      ++DI;
> +    }
>   }
>
>   BB = Emitter.getBlock();
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Wed Mar 24 20:38:16 2010
> @@ -598,8 +598,10 @@
>   // Remove the ordering of this node.
>   Ordering->remove(N);
>
> -  // And its entry in the debug info table, if any.
> -  DbgInfo->remove(N);
> +  // If any of the SDDbgValue nodes refer to this SDNode, invalidate them.
> +  SmallVector<SDDbgValue*, 2> &DbgVals = DbgInfo->getSDDbgValues(N);
> +  for (unsigned i = 0, e = DbgVals.size(); i != e; ++i)
> +    DbgVals[i]->setIsInvalidated();
>  }
>
>  /// RemoveNodeFromCSEMaps - Take the specified node out of the CSE map that
> @@ -811,6 +813,7 @@
>  SelectionDAG::~SelectionDAG() {
>   allnodes_clear();
>   delete Ordering;
> +  DbgInfo->clear();
>   delete DbgInfo;
>  }
>
> @@ -5241,24 +5244,12 @@
>   return Ordering->getOrder(SD);
>  }
>
> -/// AssignDbgInfo - Assign debug info to the SDNode.
> -void SelectionDAG::AssignDbgInfo(SDNode* SD, SDDbgValue* db) {
> -  assert(SD && "Trying to assign dbg info to a null node!");
> -  DbgInfo->add(SD, db);
> -  SD->setHasDebugValue(true);
> -}
> -
> -/// RememberDbgInfo - Remember debug info which is not assigned to an SDNode.
> -void SelectionDAG::RememberDbgInfo(SDDbgValue* db) {
> -  DbgInfo->add(db);
> -}
> -
> -/// GetDbgInfo - Get the debug info, if any, for the SDNode.
> -SDDbgValue* SelectionDAG::GetDbgInfo(const SDNode *SD) {
> -  assert(SD && "Trying to get the order of a null node!");
> -  if (SD->getHasDebugValue())
> -    return DbgInfo->getSDDbgValue(SD);
> -  return 0;
> +/// AddDbgValue - Add a dbg_value SDNode. If SD is non-null that means the
> +/// value is produced by SD.
> +void SelectionDAG::AddDbgValue(SDDbgValue *DB, SDNode *SD) {
> +  DbgInfo->add(DB, SD);
> +  if (SD)
> +    SD->setHasDebugValue(true);
>  }
>
>  //===----------------------------------------------------------------------===//
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=99469&r1=99468&r2=99469&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed Mar 24 20:38:16 2010
> @@ -3825,20 +3825,20 @@
>     ++SDNodeOrder;
>     if (isa<ConstantInt>(V) || isa<ConstantFP>(V)) {
>       SDDbgValue* dv = new SDDbgValue(Variable, V, Offset, dl, SDNodeOrder);
> -      DAG.RememberDbgInfo(dv);
> +      DAG.AddDbgValue(dv);
>     } else {
>       SDValue &N = NodeMap[V];
>       if (N.getNode()) {
>         SDDbgValue *dv = new SDDbgValue(Variable, N.getNode(),
>                                         N.getResNo(), Offset, dl, SDNodeOrder);
> -        DAG.AssignDbgInfo(N.getNode(), dv);
> +        DAG.AddDbgValue(dv, N.getNode());
>       } else {
>         // We may expand this to cover more cases.  One case where we have no
>         // data available is an unreferenced parameter; we need this fallback.
>         SDDbgValue* dv = new SDDbgValue(Variable,
>                                         UndefValue::get(V->getType()),
>                                         Offset, dl, SDNodeOrder);
> -        DAG.RememberDbgInfo(dv);
> +        DAG.AddDbgValue(dv);
>       }
>     }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>




More information about the llvm-commits mailing list