diff --git a/lib/CodeGen/StackSlotColoring.cpp b/lib/CodeGen/StackSlotColoring.cpp index 9a9b8897ee5..e1b63680e02 100644 --- a/lib/CodeGen/StackSlotColoring.cpp +++ b/lib/CodeGen/StackSlotColoring.cpp @@ -53,8 +53,11 @@ namespace { // SSIntervals - Spill slot intervals. std::vector SSIntervals; - // SSRefs - Keep a list of frame index references for each spill slot. - SmallVector, 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, 16> SSRefs; // OrigAlignments - Alignments of stack objects before coloring. SmallVector 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 &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(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 &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 &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 &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; } diff --git a/test/CodeGen/SystemZ/spill-01.ll b/test/CodeGen/SystemZ/spill-01.ll index a5e48eefcc8..b2d9fe7be4a 100644 --- a/test/CodeGen/SystemZ/spill-01.ll +++ b/test/CodeGen/SystemZ/spill-01.ll @@ -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 +}