Commit 4eb45a05 authored by Jeremy Morse's avatar Jeremy Morse Committed by Hans Wennborg
Browse files

Revert "[DebugInfo] Remove some users of DBG_VALUEs IsIndirect field" 

This reverts commit ed29dbaa.

I'm backing out D68945, which as the discussion for D73526 shows, doesn't
seem to handle the -O0 path through the codegen backend correctly. I'll
reland the patch when a fix is worked out, apologies for all the churn.
The two parent commits are part of this revert too.

Conflicts:
	llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
	llvm/test/DebugInfo/X86/dbg-addr-dse.ll

SelectionDAGBuilder conflict is due to a nearby change in e39e2b4a
that's technically unrelated. dbg-addr-dse.ll conflicted because
41206b61 (legitimately) changes the order of two lines.

There are further modifications to dbg-value-func-arg.ll: it landed after
the patch being reverted, and I've converted indirection to be represented
by the isIndirect field rather than DW_OP_deref.

(cherry picked from commit 6531a78a)
parent 04d7337d

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

+1 −1
Original line number Diff line number Diff line
@@ -1385,7 +1385,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.buildDirectDbgValue(0, DI.getVariable(), DI.getExpression());

      MIRBuilder.buildIndirectDbgValue(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

+4 −12
Original line number Diff line number Diff line
@@ -107,13 +107,9 @@ 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*/ false, Reg, Variable, DIExpr));

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

}



MachineInstrBuilder MachineIRBuilder::buildFIDbgValue(int FI,
@@ -124,15 +120,11 @@ 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)

      .addReg(0)

      .addImm(0)

      .addMetadata(Variable)

      .addMetadata(DIExpr);

      .addMetadata(Expr);

}



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

  }



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

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

}



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

llvm/lib/CodeGen/LiveDebugVariables.cpp

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

class DbgValueLocation {

public:

  DbgValueLocation(unsigned LocNo)

      : LocNo(LocNo) {

  DbgValueLocation(unsigned LocNo, bool WasIndirect)

      : LocNo(LocNo), WasIndirect(WasIndirect) {

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

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

  }



  DbgValueLocation() : LocNo(0) {}

  DbgValueLocation() : LocNo(0), WasIndirect(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);

    return DbgValueLocation(NewLocNo, WasIndirect);

  }



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

                                const DbgValueLocation &RHS) {

    return LHS.LocNo == RHS.LocNo;

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

  }



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



private:

  unsigned LocNo;

  unsigned LocNo : 31;

  unsigned WasIndirect : 1;

};



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



  /// Add a definition point to this value.

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

    DbgValueLocation Loc(getLocationNo(LocMO));

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

    DbgValueLocation Loc(getLocationNo(LocMO), IsIndirect);

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

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

    if (!I.valid() || I.start() != Idx)
@@ -319,10 +321,11 @@ 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,

      LiveInterval *LI, unsigned LocNo, bool WasIndirect,

      const SmallVectorImpl<SlotIndex> &Kills,

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

      MachineRegisterInfo &MRI, LiveIntervals &LIS);
@@ -542,6 +545,8 @@ 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) {
@@ -650,18 +655,19 @@ bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) { 
  }



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

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

                                      "LiveDebugVariables");

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

  if (IsIndirect)

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

  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));

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

  else {

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

    MO.setIsDebug();

    UV->addDef(Idx, MO);

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

  }

  return true;

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



void UserValue::addDefsFromCopies(

    LiveInterval *LI, unsigned LocNo,

    LiveInterval *LI, unsigned LocNo, bool WasIndirect,

    const SmallVectorImpl<SlotIndex> &Kills,

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

    MachineRegisterInfo &MRI, LiveIntervals &LIS) {
@@ -833,7 +839,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);

      DbgValueLocation NewLoc(LocNo, WasIndirect);

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

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

      break;
@@ -881,7 +887,8 @@ 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(), Kills, Defs, MRI, LIS);

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

                          LIS);

      continue;

    }
@@ -1323,14 +1330,21 @@ 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;

  if (Spilled)

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

  uint8_t DIExprFlags = DIExpression::ApplyOffset;

  bool IsIndirect = Loc.wasIndirect();

  if (Spilled) {

    if (IsIndirect)

      DIExprFlags |= DIExpression::DerefAfter;

    Expr =

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

    IsIndirect = true;

  }



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



  do {

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

            Spilled, MO, Variable, Expr);

            IsIndirect, 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

+5 −7
Original line number Diff line number Diff line
@@ -1393,11 +1393,9 @@ 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*/ false,

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

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

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

    } else {

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

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

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

            .addCImm(CI)

            .addReg(0U)

            .addImm(0U)

            .addMetadata(DI->getVariable())

            .addMetadata(DI->getExpression());

      else

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

            .addImm(CI->getZExtValue())

            .addReg(0U)

            .addImm(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)

          .addReg(0U)

          .addImm(0U)

          .addMetadata(DI->getVariable())

          .addMetadata(DI->getExpression());

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

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

+9 −8
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();

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

  MDNode *Expr = SD->getExpression();

  DebugLoc DL = SD->getDebugLoc();

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

         "Expected inlined-at fields to agree");
@@ -701,10 +701,11 @@ 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())

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



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

      FrameMI.addImm(0);

    else

      // Push fi onto the DIExpression stack.

      FrameMI.addReg(0);

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

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


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

  if (SD->isIndirect())

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



    MIB.addImm(0U);

  else

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



  MIB.addMetadata(Var);