Skip to content

[CodeGen] Allow multiple location for the same CSR. #168531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+155 −104
Open

[CodeGen] Allow multiple location for the same CSR. #168531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 155 additions & 102 deletions llvm/lib/CodeGen/CFIInstrInserter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,49 @@ class CFIInstrInserter : public MachineFunctionPass {
return insertedCFI;
}

private:
private:
#define INVALID_REG UINT_MAX
#define INVALID_OFFSET INT_MAX
/// contains the location where CSR register is saved.
struct CSRSavedLocation {
enum Kind { INVALID, REGISTER, CFA_OFFSET };
CSRSavedLocation() {
K = Kind::INVALID;
Reg = 0;
Offset = 0;
}
Kind K;
// Dwarf register number
unsigned Reg;
// CFA offset
int64_t Offset;
bool isValid() const { return K != Kind::INVALID; }
bool operator==(const CSRSavedLocation &RHS) const {
switch (K) {
case Kind::INVALID:
return !RHS.isValid();
case Kind::REGISTER:
return Reg == RHS.Reg;
case Kind::CFA_OFFSET:
return Offset == RHS.Offset;
}
llvm_unreachable("Unknown CSRSavedLocation Kind!");
}
void dump(raw_ostream &OS) const {
switch (K) {
case Kind::INVALID:
OS << "INVALID";
break;
case Kind::REGISTER:
OS << "In Dwarf register: " << Reg;
break;
case Kind::CFA_OFFSET:
OS << "At CFA offset: " << Offset;
break;
}
}
};

struct MBBCFAInfo {
MachineBasicBlock *MBB;
/// Value of cfa offset valid at basic block entry.
Expand All @@ -76,38 +118,27 @@ class CFIInstrInserter : public MachineFunctionPass {
unsigned IncomingCFARegister = 0;
/// Value of cfa register valid at basic block exit.
unsigned OutgoingCFARegister = 0;
/// Set of callee saved registers saved at basic block entry.
BitVector IncomingCSRSaved;
/// Set of callee saved registers saved at basic block exit.
BitVector OutgoingCSRSaved;
/// Set of locations where the callee saved registers are at basic block
/// entry.
SmallVector<CSRSavedLocation> IncomingCSRLocations;
/// Set of locations where the callee saved registers are at basic block
/// exit.
SmallVector<CSRSavedLocation> OutgoingCSRLocations;
/// If in/out cfa offset and register values for this block have already
/// been set or not.
bool Processed = false;
};

#define INVALID_REG UINT_MAX
#define INVALID_OFFSET INT_MAX
/// contains the location where CSR register is saved.
struct CSRSavedLocation {
CSRSavedLocation(std::optional<unsigned> R, std::optional<int> O)
: Reg(R), Offset(O) {}
std::optional<unsigned> Reg;
std::optional<int> Offset;
};

/// Contains cfa offset and register values valid at entry and exit of basic
/// blocks.
std::vector<MBBCFAInfo> MBBVector;

/// Map the callee save registers to the locations where they are saved.
SmallDenseMap<unsigned, CSRSavedLocation, 16> CSRLocMap;

/// Calculate cfa offset and register values valid at entry and exit for all
/// basic blocks in a function.
void calculateCFAInfo(MachineFunction &MF);
/// Calculate cfa offset and register values valid at basic block exit by
/// checking the block for CFI instructions. Block's incoming CFA info remains
/// the same.
/// checking the block for CFI instructions. Block's incoming CFA info
/// remains the same.
void calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo);
/// Update in/out cfa offset and register values for successors of the basic
/// block.
Expand All @@ -119,8 +150,8 @@ class CFIInstrInserter : public MachineFunctionPass {
/// that block.
bool insertCFIInstrs(MachineFunction &MF);
/// Return the cfa offset value that should be set at the beginning of a MBB
/// if needed. The negated value is needed when creating CFI instructions that
/// set absolute offset.
/// if needed. The negated value is needed when creating CFI instructions
/// that set absolute offset.
int64_t getCorrectCFAOffset(MachineBasicBlock *MBB) {
return MBBVector[MBB->getNumber()].IncomingCFAOffset;
}
Expand Down Expand Up @@ -162,10 +193,18 @@ void CFIInstrInserter::calculateCFAInfo(MachineFunction &MF) {
MBBInfo.OutgoingCFAOffset = InitialOffset;
MBBInfo.IncomingCFARegister = DwarfInitialRegister;
MBBInfo.OutgoingCFARegister = DwarfInitialRegister;
MBBInfo.IncomingCSRSaved.resize(NumRegs);
MBBInfo.OutgoingCSRSaved.resize(NumRegs);
MBBInfo.IncomingCSRLocations.resize(NumRegs);
MBBInfo.OutgoingCSRLocations.resize(NumRegs);
}

// Record the initial location of all registers.
MBBCFAInfo &EntryMBBInfo = MBBVector[MF.front().getNumber()];
const MCPhysReg *CSRegs = MF.getRegInfo().getCalleeSavedRegs();
for (int i = 0; CSRegs[i]; ++i) {
unsigned Reg = TRI.getDwarfRegNum(CSRegs[i], true);
CSRSavedLocation &CSRLoc = EntryMBBInfo.IncomingCSRLocations[Reg];
CSRLoc.Reg = Reg;
}
CSRLocMap.clear();

// Set in/out cfa info for all blocks in the function. This traversal is based
// on the assumption that the first block in the function is the entry block
Expand All @@ -176,14 +215,18 @@ void CFIInstrInserter::calculateCFAInfo(MachineFunction &MF) {

void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
// Outgoing cfa offset set by the block.
int64_t SetOffset = MBBInfo.IncomingCFAOffset;
int64_t &OutgoingCFAOffset = MBBInfo.OutgoingCFAOffset;
OutgoingCFAOffset = MBBInfo.IncomingCFAOffset;
// Outgoing cfa register set by the block.
unsigned SetRegister = MBBInfo.IncomingCFARegister;
unsigned &OutgoingCFARegister = MBBInfo.OutgoingCFARegister;
OutgoingCFARegister = MBBInfo.IncomingCFARegister;
// Outgoing locations for each callee-saved register set by the block.
SmallVector<CSRSavedLocation> &OutgoingCSRLocations =
MBBInfo.OutgoingCSRLocations;
OutgoingCSRLocations = MBBInfo.IncomingCSRLocations;

MachineFunction *MF = MBBInfo.MBB->getParent();
const std::vector<MCCFIInstruction> &Instrs = MF->getFrameInstructions();
const TargetRegisterInfo &TRI = *MF->getSubtarget().getRegisterInfo();
unsigned NumRegs = TRI.getNumSupportedRegs(*MF);
BitVector CSRSaved(NumRegs), CSRRestored(NumRegs);

#ifndef NDEBUG
int RememberState = 0;
Expand All @@ -192,36 +235,51 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
// Determine cfa offset and register set by the block.
for (MachineInstr &MI : *MBBInfo.MBB) {
if (MI.isCFIInstruction()) {
std::optional<unsigned> CSRReg;
std::optional<int64_t> CSROffset;
unsigned CFIIndex = MI.getOperand(0).getCFIIndex();
const MCCFIInstruction &CFI = Instrs[CFIIndex];
switch (CFI.getOperation()) {
case MCCFIInstruction::OpDefCfaRegister:
SetRegister = CFI.getRegister();
case MCCFIInstruction::OpDefCfaRegister: {
OutgoingCFARegister = CFI.getRegister();
break;
case MCCFIInstruction::OpDefCfaOffset:
SetOffset = CFI.getOffset();
}
case MCCFIInstruction::OpDefCfaOffset: {
OutgoingCFAOffset = CFI.getOffset();
break;
case MCCFIInstruction::OpAdjustCfaOffset:
SetOffset += CFI.getOffset();
}
case MCCFIInstruction::OpAdjustCfaOffset: {
OutgoingCFAOffset += CFI.getOffset();
break;
case MCCFIInstruction::OpDefCfa:
SetRegister = CFI.getRegister();
SetOffset = CFI.getOffset();
}
case MCCFIInstruction::OpDefCfa: {
OutgoingCFARegister = CFI.getRegister();
OutgoingCFAOffset = CFI.getOffset();
break;
case MCCFIInstruction::OpOffset:
CSROffset = CFI.getOffset();
}
case MCCFIInstruction::OpOffset: {
CSRSavedLocation &CSRLocation = OutgoingCSRLocations[CFI.getRegister()];
CSRLocation.K = CSRSavedLocation::Kind::CFA_OFFSET;
CSRLocation.Offset = CFI.getOffset();
break;
case MCCFIInstruction::OpRegister:
CSRReg = CFI.getRegister2();
}
case MCCFIInstruction::OpRegister: {
CSRSavedLocation &CSRLocation = OutgoingCSRLocations[CFI.getRegister()];
CSRLocation.K = CSRSavedLocation::Kind::REGISTER;
CSRLocation.Reg = CFI.getRegister2();
break;
case MCCFIInstruction::OpRelOffset:
CSROffset = CFI.getOffset() - SetOffset;
}
case MCCFIInstruction::OpRelOffset: {
CSRSavedLocation &CSRLocation = OutgoingCSRLocations[CFI.getRegister()];
CSRLocation.K = CSRSavedLocation::Kind::CFA_OFFSET;
CSRLocation.Offset = CFI.getOffset() - OutgoingCFAOffset;
break;
case MCCFIInstruction::OpRestore:
CSRRestored.set(CFI.getRegister());
}
case MCCFIInstruction::OpRestore: {
unsigned Reg = CFI.getRegister();
CSRSavedLocation &CSRLocation = OutgoingCSRLocations[Reg];
CSRLocation.K = CSRSavedLocation::Kind::REGISTER;
CSRLocation.Reg = Reg;
break;
}
case MCCFIInstruction::OpLLVMDefAspaceCfa:
// TODO: Add support for handling cfi_def_aspace_cfa.
#ifndef NDEBUG
Expand Down Expand Up @@ -266,16 +324,6 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
case MCCFIInstruction::OpValOffset:
break;
}
if (CSRReg || CSROffset) {
auto It = CSRLocMap.find(CFI.getRegister());
if (It == CSRLocMap.end()) {
CSRLocMap.insert(
{CFI.getRegister(), CSRSavedLocation(CSRReg, CSROffset)});
} else if (It->second.Reg != CSRReg || It->second.Offset != CSROffset) {
llvm_unreachable("Different saved locations for the same CSR");
}
CSRSaved.set(CFI.getRegister());
}
}
}

Expand All @@ -288,15 +336,6 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
#endif

MBBInfo.Processed = true;

// Update outgoing CFA info.
MBBInfo.OutgoingCFAOffset = SetOffset;
MBBInfo.OutgoingCFARegister = SetRegister;

// Update outgoing CSR info.
BitVector::apply([](auto x, auto y, auto z) { return (x | y) & ~z; },
MBBInfo.OutgoingCSRSaved, MBBInfo.IncomingCSRSaved, CSRSaved,
CSRRestored);
}

void CFIInstrInserter::updateSuccCFAInfo(MBBCFAInfo &MBBInfo) {
Expand All @@ -312,7 +351,7 @@ void CFIInstrInserter::updateSuccCFAInfo(MBBCFAInfo &MBBInfo) {
if (!SuccInfo.Processed) {
SuccInfo.IncomingCFAOffset = CurrentInfo.OutgoingCFAOffset;
SuccInfo.IncomingCFARegister = CurrentInfo.OutgoingCFARegister;
SuccInfo.IncomingCSRSaved = CurrentInfo.OutgoingCSRSaved;
SuccInfo.IncomingCSRLocations = CurrentInfo.OutgoingCSRLocations;
Stack.push_back(Succ);
}
}
Expand All @@ -324,7 +363,6 @@ bool CFIInstrInserter::insertCFIInstrs(MachineFunction &MF) {
const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
bool InsertedCFIInstr = false;

BitVector SetDifference;
for (MachineBasicBlock &MBB : MF) {
// Skip the first MBB in a function
if (MBB.getNumber() == MF.front().getNumber()) continue;
Expand Down Expand Up @@ -376,32 +414,34 @@ bool CFIInstrInserter::insertCFIInstrs(MachineFunction &MF) {
continue;
}

BitVector::apply([](auto x, auto y) { return x & ~y; }, SetDifference,
PrevMBBInfo->OutgoingCSRSaved, MBBInfo.IncomingCSRSaved);
for (int Reg : SetDifference.set_bits()) {
unsigned CFIIndex =
MF.addFrameInst(MCCFIInstruction::createRestore(nullptr, Reg));
BuildMI(*MBBInfo.MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex);
InsertedCFIInstr = true;
}

BitVector::apply([](auto x, auto y) { return x & ~y; }, SetDifference,
MBBInfo.IncomingCSRSaved, PrevMBBInfo->OutgoingCSRSaved);
for (int Reg : SetDifference.set_bits()) {
auto it = CSRLocMap.find(Reg);
assert(it != CSRLocMap.end() && "Reg should have an entry in CSRLocMap");
unsigned CFIIndex;
CSRSavedLocation RO = it->second;
if (!RO.Reg && RO.Offset) {
for (unsigned i = 0; i < PrevMBBInfo->OutgoingCSRLocations.size(); ++i) {
const CSRSavedLocation &PrevOutgoingCSRLoc =
PrevMBBInfo->OutgoingCSRLocations[i];
const CSRSavedLocation &HasToBeCSRLoc = MBBInfo.IncomingCSRLocations[i];
// Ignore non-callee-saved registers, they remain uninitialized (invalid).
if (!HasToBeCSRLoc.isValid())
continue;
if (HasToBeCSRLoc == PrevOutgoingCSRLoc)
continue;

unsigned CFIIndex = (unsigned)(-1);
if (HasToBeCSRLoc.K == CSRSavedLocation::Kind::CFA_OFFSET &&
HasToBeCSRLoc.Offset != PrevOutgoingCSRLoc.Offset) {
CFIIndex = MF.addFrameInst(
MCCFIInstruction::createOffset(nullptr, Reg, *RO.Offset));
} else if (RO.Reg && !RO.Offset) {
CFIIndex = MF.addFrameInst(
MCCFIInstruction::createRegister(nullptr, Reg, *RO.Reg));
} else {
llvm_unreachable("RO.Reg and RO.Offset cannot both be valid/invalid");
}
MCCFIInstruction::createOffset(nullptr, i, HasToBeCSRLoc.Offset));
} else if (HasToBeCSRLoc.K == CSRSavedLocation::Kind::REGISTER &&
(HasToBeCSRLoc.Reg != PrevOutgoingCSRLoc.Reg)) {
unsigned NewReg = HasToBeCSRLoc.Reg;
unsigned DwarfEHReg = i;
if (NewReg == DwarfEHReg) {
CFIIndex = MF.addFrameInst(
MCCFIInstruction::createRestore(nullptr, DwarfEHReg));
} else {
CFIIndex = MF.addFrameInst(
MCCFIInstruction::createRegister(nullptr, i, HasToBeCSRLoc.Reg));
}
} else
llvm_unreachable("Unexpected CSR location.");
BuildMI(*MBBInfo.MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex);
InsertedCFIInstr = true;
Expand Down Expand Up @@ -434,13 +474,23 @@ void CFIInstrInserter::reportCSRError(const MBBCFAInfo &Pred,
<< Pred.MBB->getParent()->getName() << " ***\n";
errs() << "Pred: " << Pred.MBB->getName() << " #" << Pred.MBB->getNumber()
<< " outgoing CSR Saved: ";
for (int Reg : Pred.OutgoingCSRSaved.set_bits())
errs() << Reg << " ";
for (const CSRSavedLocation &OutgoingCSRLocation :
Pred.OutgoingCSRLocations) {
if (OutgoingCSRLocation.isValid()) {
OutgoingCSRLocation.dump(errs());
errs() << " ";
}
}
errs() << "\n";
errs() << "Succ: " << Succ.MBB->getName() << " #" << Succ.MBB->getNumber()
<< " incoming CSR Saved: ";
for (int Reg : Succ.IncomingCSRSaved.set_bits())
errs() << Reg << " ";
for (const CSRSavedLocation &IncomingCSRLocation :
Succ.IncomingCSRLocations) {
if (IncomingCSRLocation.isValid()) {
IncomingCSRLocation.dump(errs());
errs() << " ";
}
}
errs() << "\n";
}

Expand All @@ -463,9 +513,12 @@ unsigned CFIInstrInserter::verify(MachineFunction &MF) {
}
// Check that IncomingCSRSaved of every successor matches the
// OutgoingCSRSaved of CurrMBB
if (SuccMBBInfo.IncomingCSRSaved != CurrMBBInfo.OutgoingCSRSaved) {
reportCSRError(CurrMBBInfo, SuccMBBInfo);
ErrorNum++;
for (unsigned i = 0; i < CurrMBBInfo.OutgoingCSRLocations.size(); ++i) {
if (!(CurrMBBInfo.OutgoingCSRLocations[i] ==
SuccMBBInfo.IncomingCSRLocations[i])) {
reportCSRError(CurrMBBInfo, SuccMBBInfo);
ErrorNum++;
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
# RUN: llc %s -mtriple=riscv64 \
# RUN: -run-pass=cfi-instr-inserter \
# RUN: -riscv-enable-cfi-instr-inserter=true
# XFAIL: *

# Technically, it is possible that a callee-saved register is saved in multiple different locations.
# CFIInstrInserter should handle this, but currently it does not.
---
name: multiple_locations
tracksRegLiveness: true
Expand Down
Loading