Skip to content

Commit

Permalink
Revert r294356, "DebugInfo: Track spilled variables in LiveDebugValues"
Browse files Browse the repository at this point in the history
It caused undefined behavior in VarLoc. As far as I investigated,

  - VarLoc::VarLoc() treats negative offset value as InvalidKind.
    Consider the case that (int64_t)MI.getOperand(1).getImm() is negative and whether it satisfies ((uint64_t)Offset < (1ULL << 32)).

  - Comparison operators in VarLoc behave undefined since VarLoc::Loc.Hash is uninitialized in case of InvalidKind.

I guess Offset (in VarLoc) could be made aware of signed, but I am not sure.
So I have reverted it for now.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294447 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chapuni committed Feb 8, 2017
1 parent 30c4561 commit 22bcc0e
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 605 deletions.
144 changes: 7 additions & 137 deletions lib/CodeGen/LiveDebugValues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,13 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/UniqueVector.h"
#include "llvm/CodeGen/LexicalScopes.h"
#include "llvm/CodeGen/MachineFrameInfo.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/Passes.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetFrameLowering.h"
#include "llvm/Target/TargetInstrInfo.h"
#include "llvm/Target/TargetLowering.h"
#include "llvm/Target/TargetRegisterInfo.h"
Expand Down Expand Up @@ -64,7 +61,6 @@ class LiveDebugValues : public MachineFunctionPass {
private:
const TargetRegisterInfo *TRI;
const TargetInstrInfo *TII;
const TargetFrameLowering *TFI;
LexicalScopes LS;

/// Keeps track of lexical scopes associated with a user value's source
Expand Down Expand Up @@ -173,11 +169,6 @@ class LiveDebugValues : public MachineFunctionPass {
typedef UniqueVector<VarLoc> VarLocMap;
typedef SparseBitVector<> VarLocSet;
typedef SmallDenseMap<const MachineBasicBlock *, VarLocSet> VarLocInMBB;
struct SpillDebugPair {
MachineInstr *SpillInst;
MachineInstr *DebugInst;
};
typedef SmallVector<SpillDebugPair, 4> SpillMap;

/// This holds the working set of currently open ranges. For fast
/// access, this is done both as a set of VarLocIDs, and a map of
Expand Down Expand Up @@ -227,21 +218,14 @@ class LiveDebugValues : public MachineFunctionPass {
}
};

bool isSpillInstruction(const MachineInstr &MI, MachineFunction *MF,
unsigned &Reg);
int extractSpillBaseRegAndOffset(const MachineInstr &MI, unsigned &Reg);

void transferDebugValue(const MachineInstr &MI, OpenRangesSet &OpenRanges,
VarLocMap &VarLocIDs);
void transferSpillInst(MachineInstr &MI, OpenRangesSet &OpenRanges,
VarLocMap &VarLocIDs, SpillMap &Spills);
void transferRegisterDef(MachineInstr &MI, OpenRangesSet &OpenRanges,
const VarLocMap &VarLocIDs);
bool transferTerminatorInst(MachineInstr &MI, OpenRangesSet &OpenRanges,
VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs);
bool transfer(MachineInstr &MI, OpenRangesSet &OpenRanges,
VarLocInMBB &OutLocs, VarLocMap &VarLocIDs, SpillMap &Spills,
bool transferSpills);
VarLocInMBB &OutLocs, VarLocMap &VarLocIDs);

bool join(MachineBasicBlock &MBB, VarLocInMBB &OutLocs, VarLocInMBB &InLocs,
const VarLocMap &VarLocIDs,
Expand Down Expand Up @@ -321,21 +305,6 @@ void LiveDebugValues::printVarLocInMBB(const MachineFunction &MF,
}
#endif

/// Given a spill instruction, extract the register and offset used to
/// address the spill location in a target independent way.
int LiveDebugValues::extractSpillBaseRegAndOffset(const MachineInstr &MI,
unsigned &Reg) {
assert(MI.hasOneMemOperand() &&
"Spill instruction does not have exactly one memory operand?");
auto MMOI = MI.memoperands_begin();
const PseudoSourceValue *PVal = (*MMOI)->getPseudoValue();
assert(PVal->kind() == PseudoSourceValue::FixedStack &&
"Inconsistent memory operand in spill instruction");
int FI = cast<FixedStackPseudoSourceValue>(PVal)->getFrameIndex();
const MachineBasicBlock *MBB = MI.getParent();
return TFI->getFrameIndexReference(*MBB->getParent(), FI, Reg);
}

/// End all previous ranges related to @MI and start a new range from @MI
/// if it is a DBG_VALUE instr.
void LiveDebugValues::transferDebugValue(const MachineInstr &MI,
Expand Down Expand Up @@ -393,85 +362,6 @@ void LiveDebugValues::transferRegisterDef(MachineInstr &MI,
OpenRanges.erase(KillSet, VarLocIDs);
}

/// Decide if @MI is a spill instruction and return true if it is. We use 2
/// criteria to make this decision:
/// - Is this instruction a store to a spill slot?
/// - Is there a register operand that is both used and killed?
bool LiveDebugValues::isSpillInstruction(const MachineInstr &MI,
MachineFunction *MF, unsigned &Reg) {
const MachineFrameInfo &FrameInfo = MF->getFrameInfo();
int FI;
const MachineMemOperand *MMO;

// To identify a spill instruction, use the same criteria as in AsmPrinter.
if (!((TII->isStoreToStackSlotPostFE(MI, FI) ||
TII->hasStoreToStackSlot(MI, MMO, FI)) &&
FrameInfo.isSpillSlotObjectIndex(FI)))
return false;

// In a spill instruction generated by the InlineSpiller the spilled register
// has its kill flag set. Return false if we don't find such a register.
Reg = 0;
for (const MachineOperand &MO : MI.operands()) {
if (MO.isReg() && MO.isUse() && MO.isKill()) {
Reg = MO.getReg();
break;
}
}
return Reg != 0;
}

/// A spilled register may indicate that we have to end the current range of
/// a variable and create a new one for the spill location.
/// We don't want to insert any instructions in transfer(), so we just create
/// the DBG_VALUE witout inserting it and keep track of it in @Spills.
/// It will be inserted into the BB when we're done iterating over the
/// instructions.
void LiveDebugValues::transferSpillInst(MachineInstr &MI,
OpenRangesSet &OpenRanges,
VarLocMap &VarLocIDs,
SpillMap &Spills) {
unsigned Reg;
MachineFunction *MF = MI.getParent()->getParent();
if (!isSpillInstruction(MI, MF, Reg))
return;

// Check if the register is the location of a debug value.
for (unsigned ID : OpenRanges.getVarLocs()) {
if (VarLocIDs[ID].isDescribedByReg() == Reg) {
DEBUG(dbgs() << "Spilling Register " << PrintReg(Reg, TRI) << '('
<< VarLocIDs[ID].Var.getVar()->getName() << ")\n");

// Create a DBG_VALUE instruction to describe the Var in its spilled
// location, but don't insert it yet to avoid invalidating the
// iterator in our caller.
unsigned SpillBase;
int SpillOffset = extractSpillBaseRegAndOffset(MI, SpillBase);
const MachineInstr *DMI = &VarLocIDs[ID].MI;
MachineInstr *SpDMI =
BuildMI(*MF, DMI->getDebugLoc(), DMI->getDesc(), true, SpillBase, 0,
DMI->getDebugVariable(), DMI->getDebugExpression());
SpDMI->getOperand(1).setImm(SpillOffset);
DEBUG(dbgs() << "Creating DBG_VALUE inst for spill: ";
SpDMI->print(dbgs(), false, TII));

// The newly created DBG_VALUE instruction SpDMI must be inserted after
// MI. Keep track of the pairing.
SpillDebugPair MIP = {&MI, SpDMI};
Spills.push_back(MIP);

// End all previous ranges of Var.
OpenRanges.erase(VarLocIDs[ID].Var);

// Add the VarLoc to OpenRanges.
VarLoc VL(*SpDMI, LS);
unsigned SpillLocID = VarLocIDs.insert(VL);
OpenRanges.insert(SpillLocID, VL.Var);
return;
}
}
}

/// Terminate all open ranges at the end of the current basic block.
bool LiveDebugValues::transferTerminatorInst(MachineInstr &MI,
OpenRangesSet &OpenRanges,
Expand All @@ -497,13 +387,10 @@ bool LiveDebugValues::transferTerminatorInst(MachineInstr &MI,

/// This routine creates OpenRanges and OutLocs.
bool LiveDebugValues::transfer(MachineInstr &MI, OpenRangesSet &OpenRanges,
VarLocInMBB &OutLocs, VarLocMap &VarLocIDs,
SpillMap &Spills, bool transferSpills) {
VarLocInMBB &OutLocs, VarLocMap &VarLocIDs) {
bool Changed = false;
transferDebugValue(MI, OpenRanges, VarLocIDs);
transferRegisterDef(MI, OpenRanges, VarLocIDs);
if (transferSpills)
transferSpillInst(MI, OpenRanges, VarLocIDs, Spills);
Changed = transferTerminatorInst(MI, OpenRanges, OutLocs, VarLocIDs);
return Changed;
}
Expand Down Expand Up @@ -592,11 +479,10 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
bool OLChanged = false;
bool MBBJoined = false;

VarLocMap VarLocIDs; // Map VarLoc<>unique ID for use in bitvectors.
VarLocMap VarLocIDs; // Map VarLoc<>unique ID for use in bitvectors.
OpenRangesSet OpenRanges; // Ranges that are open until end of bb.
VarLocInMBB OutLocs; // Ranges that exist beyond bb.
VarLocInMBB InLocs; // Ranges that are incoming after joining.
SpillMap Spills; // DBG_VALUEs associated with spills.
VarLocInMBB OutLocs; // Ranges that exist beyond bb.
VarLocInMBB InLocs; // Ranges that are incoming after joining.

DenseMap<unsigned int, MachineBasicBlock *> OrderToBB;
DenseMap<MachineBasicBlock *, unsigned int> BBToOrder;
Expand All @@ -608,14 +494,9 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
Pending;

// Initialize every mbb with OutLocs.
// We are not looking at any spill instructions during the initial pass
// over the BBs. The LiveDebugVariables pass has already created DBG_VALUE
// instructions for spills of registers that are known to be user variables
// within the BB in which the spill occurs.
for (auto &MBB : MF)
for (auto &MI : MBB)
transfer(MI, OpenRanges, OutLocs, VarLocIDs, Spills,
/*transferSpills=*/false);
transfer(MI, OpenRanges, OutLocs, VarLocIDs);

DEBUG(printVarLocInMBB(MF, OutLocs, VarLocIDs, "OutLocs after initialization",
dbgs()));
Expand Down Expand Up @@ -647,18 +528,8 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
if (MBBJoined) {
MBBJoined = false;
Changed = true;
// Now that we have started to extend ranges across BBs we need to
// examine spill instructions to see whether they spill registers that
// correspond to user variables.
for (auto &MI : *MBB)
OLChanged |= transfer(MI, OpenRanges, OutLocs, VarLocIDs, Spills,
/*transferSpills=*/true);

// Add any DBG_VALUE instructions necessitated by spills.
for (auto &SP : Spills)
MBB->insertAfter(MachineBasicBlock::iterator(*SP.SpillInst),
SP.DebugInst);
Spills.clear();
OLChanged |= transfer(MI, OpenRanges, OutLocs, VarLocIDs);

DEBUG(printVarLocInMBB(MF, OutLocs, VarLocIDs,
"OutLocs after propagating", dbgs()));
Expand Down Expand Up @@ -692,7 +563,6 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {

TRI = MF.getSubtarget().getRegisterInfo();
TII = MF.getSubtarget().getInstrInfo();
TFI = MF.getSubtarget().getFrameLowering();
LS.initialize(MF);

bool Changed = ExtendRanges(MF);
Expand Down
Loading

0 comments on commit 22bcc0e

Please sign in to comment.