mirror of
https://github.com/RPCS3/llvm.git
synced 2024-12-16 08:29:43 +00:00
Fix double renaming bug in stack coloring pass
The stack coloring pass renumbered frame indexes with a loop of the form:
for each frame index FI
for each instruction I that uses FI
for each use of FI in I
rename FI to FI'
This caused problems if an instruction used two frame indexes F0 and F1
and if F0 was renamed to F1 and F1 to F2. The first time we visited the
instruction we changed F0 to F1, then we changed both F1s to F2.
In other words, the problem was that SSRefs recorded which instructions
used an FI, but not which MachineOperands and MachineMemOperands within
that instruction used it.
This is easily fixed for MachineOperands by walking the instructions
once and processing each operand in turn. There's already a loop to
do that for dead store elimination, so it seemed more efficient to
fuse the two at the block level.
MachineMemOperands are more tricky because they can be shared between
instructions. The patch handles them by making SSRefs an array of
MachineMemOperands rather than an array of MachineInstrs. We might end
up processing the same MachineMemOperand twice, but that's OK because
we always know from the SSRefs index what the original frame index was.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@185703 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Richard Sandiford
parent
cf1b5bd60a
commit
5f1c7e5eac
@ -53,8 +53,11 @@ namespace {
|
||||
// SSIntervals - Spill slot intervals.
|
||||
std::vector<LiveInterval*> SSIntervals;
|
||||
|
||||
// SSRefs - Keep a list of frame index references for each spill slot.
|
||||
SmallVector<SmallVector<MachineInstr*, 8>, 16> SSRefs;
|
||||
// SSRefs - Keep a list of MachineMemOperands for each spill slot.
|
||||
// MachineMemOperands can be shared between instructions, so we need
|
||||
// to be careful that renames like [FI0, FI1] -> [FI1, FI2] do not
|
||||
// become FI0 -> FI1 -> FI2.
|
||||
SmallVector<SmallVector<MachineMemOperand *, 8>, 16> SSRefs;
|
||||
|
||||
// OrigAlignments - Alignments of stack objects before coloring.
|
||||
SmallVector<unsigned, 16> OrigAlignments;
|
||||
@ -103,7 +106,7 @@ namespace {
|
||||
bool OverlapWithAssignments(LiveInterval *li, int Color) const;
|
||||
int ColorSlot(LiveInterval *li);
|
||||
bool ColorSlots(MachineFunction &MF);
|
||||
void RewriteInstruction(MachineInstr *MI, int OldFI, int NewFI,
|
||||
void RewriteInstruction(MachineInstr *MI, SmallVector<int, 16> &SlotMapping,
|
||||
MachineFunction &MF);
|
||||
bool RemoveDeadStores(MachineBasicBlock* MBB);
|
||||
};
|
||||
@ -155,7 +158,18 @@ void StackSlotColoring::ScanForSpillSlotRefs(MachineFunction &MF) {
|
||||
LiveInterval &li = LS->getInterval(FI);
|
||||
if (!MI->isDebugValue())
|
||||
li.weight += LiveIntervals::getSpillWeight(false, true, Freq);
|
||||
SSRefs[FI].push_back(MI);
|
||||
}
|
||||
for (MachineInstr::mmo_iterator MMOI = MI->memoperands_begin(),
|
||||
EE = MI->memoperands_end(); MMOI != EE; ++MMOI) {
|
||||
MachineMemOperand *MMO = *MMOI;
|
||||
if (const Value *V = MMO->getValue()) {
|
||||
if (const FixedStackPseudoSourceValue *FSV =
|
||||
dyn_cast<FixedStackPseudoSourceValue>(V)) {
|
||||
int FI = FSV->getFrameIndex();
|
||||
if (FI >= 0)
|
||||
SSRefs[FI].push_back(MMO);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -291,15 +305,26 @@ bool StackSlotColoring::ColorSlots(MachineFunction &MF) {
|
||||
if (!Changed)
|
||||
return false;
|
||||
|
||||
// Rewrite all MO_FrameIndex operands.
|
||||
// Rewrite all MachineMemOperands.
|
||||
for (unsigned SS = 0, SE = SSRefs.size(); SS != SE; ++SS) {
|
||||
int NewFI = SlotMapping[SS];
|
||||
if (NewFI == -1 || (NewFI == (int)SS))
|
||||
continue;
|
||||
|
||||
SmallVectorImpl<MachineInstr*> &RefMIs = SSRefs[SS];
|
||||
for (unsigned i = 0, e = RefMIs.size(); i != e; ++i)
|
||||
RewriteInstruction(RefMIs[i], SS, NewFI, MF);
|
||||
const Value *NewSV = PseudoSourceValue::getFixedStack(NewFI);
|
||||
SmallVectorImpl<MachineMemOperand *> &RefMMOs = SSRefs[SS];
|
||||
for (unsigned i = 0, e = RefMMOs.size(); i != e; ++i)
|
||||
RefMMOs[i]->setValue(NewSV);
|
||||
}
|
||||
|
||||
// Rewrite all MO_FrameIndex operands. Look for dead stores.
|
||||
for (MachineFunction::iterator MBBI = MF.begin(), E = MF.end();
|
||||
MBBI != E; ++MBBI) {
|
||||
MachineBasicBlock *MBB = &*MBBI;
|
||||
for (MachineBasicBlock::iterator MII = MBB->begin(), EE = MBB->end();
|
||||
MII != EE; ++MII)
|
||||
RewriteInstruction(MII, SlotMapping, MF);
|
||||
RemoveDeadStores(MBB);
|
||||
}
|
||||
|
||||
// Delete unused stack slots.
|
||||
@ -314,28 +339,24 @@ bool StackSlotColoring::ColorSlots(MachineFunction &MF) {
|
||||
|
||||
/// RewriteInstruction - Rewrite specified instruction by replacing references
|
||||
/// to old frame index with new one.
|
||||
void StackSlotColoring::RewriteInstruction(MachineInstr *MI, int OldFI,
|
||||
int NewFI, MachineFunction &MF) {
|
||||
void StackSlotColoring::RewriteInstruction(MachineInstr *MI,
|
||||
SmallVector<int, 16> &SlotMapping,
|
||||
MachineFunction &MF) {
|
||||
// Update the operands.
|
||||
for (unsigned i = 0, ee = MI->getNumOperands(); i != ee; ++i) {
|
||||
MachineOperand &MO = MI->getOperand(i);
|
||||
if (!MO.isFI())
|
||||
continue;
|
||||
int FI = MO.getIndex();
|
||||
if (FI != OldFI)
|
||||
int OldFI = MO.getIndex();
|
||||
if (OldFI < 0)
|
||||
continue;
|
||||
int NewFI = SlotMapping[OldFI];
|
||||
if (NewFI == -1 || NewFI == OldFI)
|
||||
continue;
|
||||
MO.setIndex(NewFI);
|
||||
}
|
||||
|
||||
// Update the memory references. This changes the MachineMemOperands
|
||||
// directly. They may be in use by multiple instructions, however all
|
||||
// instructions using OldFI are being rewritten to use NewFI.
|
||||
const Value *OldSV = PseudoSourceValue::getFixedStack(OldFI);
|
||||
const Value *NewSV = PseudoSourceValue::getFixedStack(NewFI);
|
||||
for (MachineInstr::mmo_iterator I = MI->memoperands_begin(),
|
||||
E = MI->memoperands_end(); I != E; ++I)
|
||||
if ((*I)->getValue() == OldSV)
|
||||
(*I)->setValue(NewSV);
|
||||
// The MachineMemOperands have already been updated.
|
||||
}
|
||||
|
||||
|
||||
@ -429,10 +450,5 @@ bool StackSlotColoring::runOnMachineFunction(MachineFunction &MF) {
|
||||
Assignments[i].clear();
|
||||
Assignments.clear();
|
||||
|
||||
if (Changed) {
|
||||
for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ++I)
|
||||
Changed |= RemoveDeadStores(I);
|
||||
}
|
||||
|
||||
return Changed;
|
||||
}
|
||||
|
||||
@ -381,3 +381,78 @@ define void @f9() {
|
||||
|
||||
ret void
|
||||
}
|
||||
|
||||
; This showed a problem with the way stack coloring updated instructions.
|
||||
; The copy from %val9 to %newval8 can be done using an MVC, which then
|
||||
; has two frame index operands. Stack coloring chose a valid renumbering
|
||||
; [FI0, FI1] -> [FI1, FI2], but applied it in the form FI0 -> FI1 -> FI2,
|
||||
; so that both operands ended up being the same.
|
||||
define void @f10() {
|
||||
; CHECK: f10:
|
||||
; CHECK: lgrl [[REG:%r[0-9]+]], h9
|
||||
; CHECK: stg [[REG]], [[VAL9:[0-9]+]](%r15)
|
||||
; CHECK: brasl %r14, foo@PLT
|
||||
; CHECK: brasl %r14, foo@PLT
|
||||
; CHECK: mvc [[NEWVAL8:[0-9]+]](8,%r15), [[VAL9]](%r15)
|
||||
; CHECK: brasl %r14, foo@PLT
|
||||
; CHECK: lg [[REG:%r[0-9]+]], [[NEWVAL8]](%r15)
|
||||
; CHECK: stgrl [[REG]], h8
|
||||
; CHECK: br %r14
|
||||
entry:
|
||||
%val0 = load volatile i64 *@h0
|
||||
%val1 = load volatile i64 *@h1
|
||||
%val2 = load volatile i64 *@h2
|
||||
%val3 = load volatile i64 *@h3
|
||||
%val4 = load volatile i64 *@h4
|
||||
%val5 = load volatile i64 *@h5
|
||||
%val6 = load volatile i64 *@h6
|
||||
%val7 = load volatile i64 *@h7
|
||||
%val8 = load volatile i64 *@h8
|
||||
%val9 = load volatile i64 *@h9
|
||||
|
||||
call void @foo()
|
||||
|
||||
store volatile i64 %val0, i64 *@h0
|
||||
store volatile i64 %val1, i64 *@h1
|
||||
store volatile i64 %val2, i64 *@h2
|
||||
store volatile i64 %val3, i64 *@h3
|
||||
store volatile i64 %val4, i64 *@h4
|
||||
store volatile i64 %val5, i64 *@h5
|
||||
store volatile i64 %val6, i64 *@h6
|
||||
store volatile i64 %val7, i64 *@h7
|
||||
|
||||
%check = load volatile i64 *@h0
|
||||
%cond = icmp eq i64 %check, 0
|
||||
br i1 %cond, label %skip, label %fallthru
|
||||
|
||||
fallthru:
|
||||
call void @foo()
|
||||
|
||||
store volatile i64 %val0, i64 *@h0
|
||||
store volatile i64 %val1, i64 *@h1
|
||||
store volatile i64 %val2, i64 *@h2
|
||||
store volatile i64 %val3, i64 *@h3
|
||||
store volatile i64 %val4, i64 *@h4
|
||||
store volatile i64 %val5, i64 *@h5
|
||||
store volatile i64 %val6, i64 *@h6
|
||||
store volatile i64 %val7, i64 *@h7
|
||||
store volatile i64 %val8, i64 *@h8
|
||||
br label %skip
|
||||
|
||||
skip:
|
||||
%newval8 = phi i64 [ %val8, %entry ], [ %val9, %fallthru ]
|
||||
call void @foo()
|
||||
|
||||
store volatile i64 %val0, i64 *@h0
|
||||
store volatile i64 %val1, i64 *@h1
|
||||
store volatile i64 %val2, i64 *@h2
|
||||
store volatile i64 %val3, i64 *@h3
|
||||
store volatile i64 %val4, i64 *@h4
|
||||
store volatile i64 %val5, i64 *@h5
|
||||
store volatile i64 %val6, i64 *@h6
|
||||
store volatile i64 %val7, i64 *@h7
|
||||
store volatile i64 %newval8, i64 *@h8
|
||||
store volatile i64 %val9, i64 *@h9
|
||||
|
||||
ret void
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user