Commit ed29dbaa authored by Jeremy Morse's avatar Jeremy Morse
Browse files

[DebugInfo] Remove some users of DBG_VALUEs IsIndirect field 

This patch kills off a significant user of the "IsIndirect" field of
DBG_VALUE machine insts. Brought up in in PR41675, IsIndirect is
techncally redundant as it can be expressed by the DIExpression of a
DBG_VALUE inst, and it isn't helpful to have two ways of expressing
things.

Rather than setting IsIndirect, have DBG_VALUE creators add an extra deref
to the insts DIExpression. There should now be no appearences of
IsIndirect=True from isel down to LiveDebugVariables / VirtRegRewriter,
which is ensured by an assertion in LDVImpl::handleDebugValue. This means
we also get to delete the IsIndirect handling in LiveDebugVariables. Tests
can be upgraded by for example swapping the following IsIndirect=True
DBG_VALUE:

  DBG_VALUE $somereg, 0, !123, !DIExpression(DW_OP_foo)

With one where the indirection is in the DIExpression, by _appending_
a deref:

  DBG_VALUE $somereg, $noreg, !123, !DIExpression(DW_OP_foo, DW_OP_deref)

Which both mean the same thing. 

Most of the test changes in this patch are updates of that form; also some
changes in how the textual assembly printer handles these insts.

Differential Revision: https://reviews.llvm.org/D68945

llvm-svn: 374877
parent 4706f3be

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

+1 −1
Original line number Diff line number Diff line
@@ -1378,7 +1378,7 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID, 
    if (!V) {

      // Currently the optimizer can produce this; insert an undef to

      // help debugging.  Probably the optimizer should not do this.

      MIRBuilder.buildIndirectDbgValue(0, DI.getVariable(), DI.getExpression());

      MIRBuilder.buildDirectDbgValue(0, DI.getVariable(), DI.getExpression());

    } else if (const auto *CI = dyn_cast<Constant>(V)) {

      MIRBuilder.buildConstDbgValue(*CI, DI.getVariable(), DI.getExpression());

    } else {

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

+12 −4
Original line number Diff line number Diff line
@@ -107,9 +107,13 @@ MachineIRBuilder::buildIndirectDbgValue(Register Reg, const MDNode *Variable, 
  assert(

      cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(getDL()) &&

      "Expected inlined-at fields to agree");

  // DBG_VALUE insts now carry IR-level indirection in their DIExpression

  // rather than encoding it in the instruction itself.

  const DIExpression *DIExpr = cast<DIExpression>(Expr);

  DIExpr = DIExpression::append(DIExpr, {dwarf::DW_OP_deref});

  return insertInstr(BuildMI(getMF(), getDL(),

                             getTII().get(TargetOpcode::DBG_VALUE),

                             /*IsIndirect*/ true, Reg, Variable, Expr));

                             /*IsIndirect*/ false, Reg, Variable, DIExpr));

}



MachineInstrBuilder MachineIRBuilder::buildFIDbgValue(int FI,
@@ -120,11 +124,15 @@ MachineInstrBuilder MachineIRBuilder::buildFIDbgValue(int FI, 
  assert(

      cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(getDL()) &&

      "Expected inlined-at fields to agree");

  // DBG_VALUE insts now carry IR-level indirection in their DIExpression

  // rather than encoding it in the instruction itself.

  const DIExpression *DIExpr = cast<DIExpression>(Expr);

  DIExpr = DIExpression::append(DIExpr, {dwarf::DW_OP_deref});

  return buildInstr(TargetOpcode::DBG_VALUE)

      .addFrameIndex(FI)

      .addImm(0)

      .addReg(0)

      .addMetadata(Variable)

      .addMetadata(Expr);

      .addMetadata(DIExpr);

}



MachineInstrBuilder MachineIRBuilder::buildConstDbgValue(const Constant &C,
@@ -148,7 +156,7 @@ MachineInstrBuilder MachineIRBuilder::buildConstDbgValue(const Constant &C, 
    MIB.addReg(0U);

  }



  return MIB.addImm(0).addMetadata(Variable).addMetadata(Expr);

  return MIB.addReg(0).addMetadata(Variable).addMetadata(Expr);

}



MachineInstrBuilder MachineIRBuilder::buildDbgLabel(const MDNode *Label) {

llvm/lib/CodeGen/LiveDebugVariables.cpp

+19 −33
Original line number Diff line number Diff line
@@ -99,28 +99,27 @@ enum : unsigned { UndefLocNo = ~0U }; 
/// usage of the location.

class DbgValueLocation {

public:

  DbgValueLocation(unsigned LocNo, bool WasIndirect)

      : LocNo(LocNo), WasIndirect(WasIndirect) {

  DbgValueLocation(unsigned LocNo)

      : LocNo(LocNo) {

    static_assert(sizeof(*this) == sizeof(unsigned), "bad bitfield packing");

    assert(locNo() == LocNo && "location truncation");

  }



  DbgValueLocation() : LocNo(0), WasIndirect(0) {}

  DbgValueLocation() : LocNo(0) {}



  unsigned locNo() const {

    // Fix up the undef location number, which gets truncated.

    return LocNo == INT_MAX ? UndefLocNo : LocNo;

  }

  bool wasIndirect() const { return WasIndirect; }

  bool isUndef() const { return locNo() == UndefLocNo; }



  DbgValueLocation changeLocNo(unsigned NewLocNo) const {

    return DbgValueLocation(NewLocNo, WasIndirect);

    return DbgValueLocation(NewLocNo);

  }



  friend inline bool operator==(const DbgValueLocation &LHS,

                                const DbgValueLocation &RHS) {

    return LHS.LocNo == RHS.LocNo && LHS.WasIndirect == RHS.WasIndirect;

    return LHS.LocNo == RHS.LocNo;

  }



  friend inline bool operator!=(const DbgValueLocation &LHS,
@@ -129,8 +128,7 @@ public: 
  }



private:

  unsigned LocNo : 31;

  unsigned WasIndirect : 1;

  unsigned LocNo;

};



/// Map of where a user value is live, and its location.
@@ -261,8 +259,8 @@ public: 
  void mapVirtRegs(LDVImpl *LDV);



  /// Add a definition point to this value.

  void addDef(SlotIndex Idx, const MachineOperand &LocMO, bool IsIndirect) {

    DbgValueLocation Loc(getLocationNo(LocMO), IsIndirect);

  void addDef(SlotIndex Idx, const MachineOperand &LocMO) {

    DbgValueLocation Loc(getLocationNo(LocMO));

    // Add a singular (Idx,Idx) -> Loc mapping.

    LocMap::iterator I = locInts.find(Idx);

    if (!I.valid() || I.start() != Idx)
@@ -297,11 +295,10 @@ public: 
  ///

  /// \param LI Scan for copies of the value in LI->reg.

  /// \param LocNo Location number of LI->reg.

  /// \param WasIndirect Indicates if the original use of LI->reg was indirect

  /// \param Kills Points where the range of LocNo could be extended.

  /// \param [in,out] NewDefs Append (Idx, LocNo) of inserted defs here.

  void addDefsFromCopies(

      LiveInterval *LI, unsigned LocNo, bool WasIndirect,

      LiveInterval *LI, unsigned LocNo,

      const SmallVectorImpl<SlotIndex> &Kills,

      SmallVectorImpl<std::pair<SlotIndex, DbgValueLocation>> &NewDefs,

      MachineRegisterInfo &MRI, LiveIntervals &LIS);
@@ -521,8 +518,6 @@ void UserValue::print(raw_ostream &OS, const TargetRegisterInfo *TRI) { 
      OS << "undef";

    else {

      OS << I.value().locNo();

      if (I.value().wasIndirect())

        OS << " ind";

    }

  }

  for (unsigned i = 0, e = locations.size(); i != e; ++i) {
@@ -631,19 +626,18 @@ bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) { 
  }



  // Get or create the UserValue for (variable,offset) here.

  bool IsIndirect = MI.getOperand(1).isImm();

  if (IsIndirect)

    assert(MI.getOperand(1).getImm() == 0 && "DBG_VALUE with nonzero offset");

  assert(!MI.getOperand(1).isImm() && "DBG_VALUE with indirect flag before "

                                      "LiveDebugVariables");

  const DILocalVariable *Var = MI.getDebugVariable();

  const DIExpression *Expr = MI.getDebugExpression();

  UserValue *UV =

      getUserValue(Var, Expr, MI.getDebugLoc());

  if (!Discard)

    UV->addDef(Idx, MI.getOperand(0), IsIndirect);

    UV->addDef(Idx, MI.getOperand(0));

  else {

    MachineOperand MO = MachineOperand::CreateReg(0U, false);

    MO.setIsDebug();

    UV->addDef(Idx, MO, false);

    UV->addDef(Idx, MO);

  }

  return true;

}
@@ -751,7 +745,7 @@ void UserValue::extendDef(SlotIndex Idx, DbgValueLocation Loc, LiveRange *LR, 
}



void UserValue::addDefsFromCopies(

    LiveInterval *LI, unsigned LocNo, bool WasIndirect,

    LiveInterval *LI, unsigned LocNo,

    const SmallVectorImpl<SlotIndex> &Kills,

    SmallVectorImpl<std::pair<SlotIndex, DbgValueLocation>> &NewDefs,

    MachineRegisterInfo &MRI, LiveIntervals &LIS) {
@@ -815,7 +809,7 @@ void UserValue::addDefsFromCopies( 
      MachineInstr *CopyMI = LIS.getInstructionFromIndex(DstVNI->def);

      assert(CopyMI && CopyMI->isCopy() && "Bad copy value");

      unsigned LocNo = getLocationNo(CopyMI->getOperand(0));

      DbgValueLocation NewLoc(LocNo, WasIndirect);

      DbgValueLocation NewLoc(LocNo);

      I.insert(Idx, Idx.getNextSlot(), NewLoc);

      NewDefs.push_back(std::make_pair(Idx, NewLoc));

      break;
@@ -863,8 +857,7 @@ void UserValue::computeIntervals(MachineRegisterInfo &MRI, 
      // sub-register in that regclass). For now, simply skip handling copies if

      // a sub-register is involved.

      if (LI && !LocMO.getSubReg())

        addDefsFromCopies(LI, Loc.locNo(), Loc.wasIndirect(), Kills, Defs, MRI,

                          LIS);

        addDefsFromCopies(LI, Loc.locNo(), Kills, Defs, MRI, LIS);

      continue;

    }
@@ -1302,21 +1295,14 @@ void UserValue::insertDebugValue(MachineBasicBlock *MBB, SlotIndex StartIdx, 
  // that the original virtual register was a pointer. Also, add the stack slot

  // offset for the spilled register to the expression.

  const DIExpression *Expr = Expression;

  uint8_t DIExprFlags = DIExpression::ApplyOffset;

  bool IsIndirect = Loc.wasIndirect();

  if (Spilled) {

    if (IsIndirect)

      DIExprFlags |= DIExpression::DerefAfter;

    Expr =

        DIExpression::prepend(Expr, DIExprFlags, SpillOffset);

    IsIndirect = true;

  }

  if (Spilled)

    Expr = DIExpression::prepend(Expr, DIExpression::ApplyOffset, SpillOffset);



  assert((!Spilled || MO.isFI()) && "a spilled location must be a frame index");



  do {

    BuildMI(*MBB, I, getDebugLoc(), TII.get(TargetOpcode::DBG_VALUE),

            IsIndirect, MO, Variable, Expr);

            Spilled, MO, Variable, Expr);



    // Continue and insert DBG_VALUES after every redefinition of register

    // associated with the debug value within the range

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

+7 −5
Original line number Diff line number Diff line
@@ -1389,9 +1389,11 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) { 
             "Expected inlined-at fields to agree");

      // A dbg.declare describes the address of a source variable, so lower it

      // into an indirect DBG_VALUE.

      auto *Expr = DI->getExpression();

      Expr = DIExpression::append(Expr, {dwarf::DW_OP_deref});

      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,

              TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ true,

              *Op, DI->getVariable(), DI->getExpression());

              TII.get(TargetOpcode::DBG_VALUE), /*IsIndirect*/ false,

              *Op, DI->getVariable(), Expr);

    } else {

      // We can't yet handle anything else here because it would require

      // generating code, thus altering codegen because of debug info.
@@ -1415,19 +1417,19 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) { 
      if (CI->getBitWidth() > 64)

        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)

            .addCImm(CI)

            .addImm(0U)

            .addReg(0U)

            .addMetadata(DI->getVariable())

            .addMetadata(DI->getExpression());

      else

        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)

            .addImm(CI->getZExtValue())

            .addImm(0U)

            .addReg(0U)

            .addMetadata(DI->getVariable())

            .addMetadata(DI->getExpression());

    } else if (const auto *CF = dyn_cast<ConstantFP>(V)) {

      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)

          .addFPImm(CF)

          .addImm(0U)

          .addReg(0U)

          .addMetadata(DI->getVariable())

          .addMetadata(DI->getExpression());

    } else if (unsigned Reg = lookUpRegForValue(V)) {

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

+8 −9
Original line number Diff line number Diff line
@@ -677,7 +677,7 @@ MachineInstr * 
InstrEmitter::EmitDbgValue(SDDbgValue *SD,

                           DenseMap<SDValue, unsigned> &VRBaseMap) {

  MDNode *Var = SD->getVariable();

  MDNode *Expr = SD->getExpression();

  const DIExpression *Expr = SD->getExpression();

  DebugLoc DL = SD->getDebugLoc();

  assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&

         "Expected inlined-at fields to agree");
@@ -701,11 +701,10 @@ InstrEmitter::EmitDbgValue(SDDbgValue *SD, 
    // EmitTargetCodeForFrameDebugValue is responsible for allocation.

    auto FrameMI = BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))

                       .addFrameIndex(SD->getFrameIx());



    if (SD->isIndirect())

      // Push [fi + 0] onto the DIExpression stack.

      FrameMI.addImm(0);

    else

      // Push fi onto the DIExpression stack.

      Expr = DIExpression::append(Expr, {dwarf::DW_OP_deref});



    FrameMI.addReg(0);

    return FrameMI.addMetadata(Var).addMetadata(Expr);

  }
@@ -752,8 +751,8 @@ InstrEmitter::EmitDbgValue(SDDbgValue *SD, 


  // Indirect addressing is indicated by an Imm as the second parameter.

  if (SD->isIndirect())

    MIB.addImm(0U);

  else

    Expr = DIExpression::append(Expr, {dwarf::DW_OP_deref});



  MIB.addReg(0U, RegState::Debug);



  MIB.addMetadata(Var);