Fix for PR 26500

[nemanjai]

Summary

The register used in emitPrologue/emitEpilogue to spill and reload the condition register is hard coded as R12. This isn't a problem in general, however when we shrink-wrap the function, the prologue/epilogue will end up in a block that may be using R12. In a case like that, we will clobber a value that is potentially needed.

This patch provides the support for acquiring an unused register for spilling the CR. In cases where there isn't an extra available register, the LR and CR will be spilled/reloaded using the same temporary register.

In the cases where we are not shrinkwrapping (i.e. the prologue/epilogue are staying put), we will not look for an available register since R12 is available.

[nemanjai]

I was thinking of adding a test case from the PR (reduced in a trivial way but still 915 lines of IR). However, I think the key things to test here are:

1. No change in behaviour for non-shrink-rapped functions
2. Shrink-wrapped functions only change by potentially using a register other than R12 to spill CR
3. The selected register isn't live in the basic block

Number 1. is accomplished by the fact that other test cases did not have to change. Numbers 2. and 3. cannot really be tested reliably in the IR test case.
The only thing I see as a possibility is that I add a test case to projects/test-suite that ensures there's no crash with this fix (i.e. build the program Anton posted in the PR and run it).

[hfinkel]

A few comments, but otherwise, LGTM. Thanks!

lib/Target/PowerPC/PPCFrameLowering.cpp

633

Variable names start with a capital letter. (I see it is like this below also, so please fix the others in a follow-up commit).

637

We need a comment somewhere explaining what's going on, and this might be a good place.

// We need a scratch register for spilling LR and for spilling CR. By default, we use two scratch registers to hide latency. However, if only one scratch register is available, we can adjust for that by not overlapping the spill code. However, if we need to realign the stack (i.e. have a base pointer) and the stack frame is large, we need two scratch registers.

867

Please add a comment here noting that this condition needs to be kept in sync with the check in canUseAsPrologue.

lib/Target/PowerPC/PPCFrameLowering.h

56

I think that the code would be easier to read if you made some of these have default parameters:

bool findScratchRegister(MachineBasicBlock *MBB,
                           bool UseAtEnd,
                           bool Needs2ScratchRegs = false,
                           unsigned *ScratchRegister = nullptr,
                           unsigned *ScratchRegisterCR = nullptr) const;

and then adjusted the call sites accordingly.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

570

Need to give ScratchRegisterCR a default value

572

No, you shouldn't do this this. You should define what you want the ScratchRegisterCR to be here, don't assume it is defined elsewhere.

647

Remove /*Needs2ScratchRegs*/ in comments

lib/Target/PowerPC/PPCFrameLowering.h

51

Please updated the doxygen comments for this function.

[qcolombet]

lib/Target/PowerPC/PPCFrameLowering.cpp

605

Don’t know if you want to fix this while you are here, but you’ll need to keep only scratch registers in the returned set.
See Tim’s comment in http://reviews.llvm.org/D17350 for more details.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

606

Yes, good catch!
This should be easy to do by masking out all non-volatile registers from the bit vector, before traversing it.

[nemanjai]

• Fixed the names
• Provided default arguments
• Removed callee-saved registers from consideration
• Provided a default register for CR spilling (rather than expecting the caller to have set it)
• Added the requested comments

[nemanjai]

I forgot one change:

• If the second scratch register is not available, find_next will return -1

Of course, this should never happen if the caller provides the correct argument for Needs2ScratchRegs (we will have returned before getting here). However, assuming the caller incorrectly sets the argument, we don't want to return some huge register number.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

597

You don't need to do this now, since you've initialized ScratchRegisterCR at the beginning.

612

Probably should expand on this comment a bit...

642

You should refactor this logic into a separate method and use it when calling findScratchRegister in emitPrologue and emitEpilogue, as well as here.

644

Need to update this call to findScratchRegister as well, since the epilogue can use two scratch registers.

737–738

Need to use the same logic when calling findScratchRegister here, as used above in canUseAsPrologue.

867

This is currently not in sync, since you're only using MustSaveCRs to indicate whether you need two scratch registers.

1089–1090

We need to make a corresponding change to findScratchRegister in canUseAsEpilogue.

lib/Target/PowerPC/PPCFrameLowering.h

46

What happens if it cannot find two registers?

[nemanjai]

lib/Target/PowerPC/PPCFrameLowering.cpp

597

Well, there's no guarantee at this point that R12 is available. If I don't do this, we can easily end up in the same situation that prompted the bug. The idea is that I assume we don't have the second scratch register available until one is found. The code that reorders the CR/LR spills depends on the two registers actually being the same.

612

How about:
We shouldn't use callee-saved registers as scratch registers as they may be available when looking for a candidate block for shrink wrapping but not available when the actual prologue/epilogue is being emitted.

642

This is a good point. If I don't have to save the CR but I do have to realign a large stack frame and R0 is available as a scratch register, findScratchRegister will just set ScratchRegisterCR to R12 without checking if it's available.

644

The code in emitEpilogue only uses the second scratch register for spilling the CR so there's never a requirement for a second scratch register. When emitting the epilogue, we will ask nicely for a second scratch register if we're spilling the CR, but if we don't get the second one, we're still functionally correct (the CR spill sequence completes before the LR sequence starts).

737–739

Yes, I agree. Well almost. The condition in words should be "if we need to spill the CR OR we need to realign a large stack frame".

867

I see what you mean. It's in sync with canUseAsPrologue but not with the call to findScratchRegister.

1089–1091

I think that this would have the potential of causing some functions not to be shrink wrapped unnecessarily. At the very least, it will make blocks that don't have two available scratch registers invalid for epilogue insertion when we technically could use the block as epilogue. This is so because as I mentioned in the comment above, the emitEpilogue function never has a hard requirement for two scratch registers - if we don't have two, we just use the same one for spilling the CR and LR.

So now the question is, do we want that? Using a single register will cause the CR/LR spills to have increased overall latency. On the other hand, if no blocks are available for epilogue insertion, we don't shrink wrap. I can implement it either way.

lib/Target/PowerPC/PPCFrameLowering.h

46

I've described that in the return section. Do you want me to add something here as well?

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

597

See comment below.

610

I don't think you want this BV.count() < 2 check here.

As it stands now, if there aren't at least 2 bits available, this will return false (indicating you can't find 2 different registers).
This ties in with the if condition above on line 601. It's possible that there is one register available, in which case both ScratchRegister and ScratchRegisterCR are set to R0 (line 604) but then this returns false here. The effect of that is shrink wrapping will be disabled, and the same temp register will be used for both the CR and LR (i.e., lost both opportunities).

I think this logic needs to be reworked here to account for the 3 different scenarios:

1. No scratch registers available: set ScratchRegister to R0, ScratchRegisterCR to R12, return false;
2. One scratch register available: set both ScratchRegisters to available reg; return true;
3. Two scratch registers available: set ScratchRegister and ScratchRegisterCR using the bit vector iterator, return true.

612

... because they have been added as live-in to the prologue block by the PrologueEpilogueInserter.

616

Why would find_next return -1? You've already checked that there are at least 2 bits set.

1089–1091

I think we should use two scratch registers, if they are available, otherwise just use one. There should be other ways we can overlap the instructions to hide the latencies, if need be.
However, I would put a comment in both canUseAsEpilogue and emitEpilogue saying that this.

lib/Target/PowerPC/PPCFrameLowering.h

46

Sorry, I missed that.
But you should update to indicate R0 and R12 are used as the defaults.

[nemanjai]

Cleaned up the code and added comments to make it a bit more clear as to what is happening. Made sure that we require two registers when we're emitting a prologue for a function that requires two registers.
I can add the following as a comment for findScratchRegister if the reviewers would like (since the semantics of that function are difficult to follow).

Just to enumerate all the possibilities for findScratchRegister:

• When querying whether a block is a valid candidate for the prologue/epilogue
  • The block is in a function where large stack frame realignment is required: we return true only if there are two available registers
  • The block is in a function where large stack frame realignment is not required: we return true if at least one register is available
  • The output parameters are not set in either case
• When requesting the actual scratch registers
  • We need to save the CR and realign a large stack frame:
    • If two registers are available, we'll set both
    • If only one register is available, we'll set both to that one (should be an impossible situation)*
    • If no registers are available, we'll set both to R0 (should be an impossible situation)*
  • We need to save the CR but do not need to realign a large stack frame:
    • If two registers are available, we'll set both
    • If only one is available, we'll set both to that one
    • If no registers are available, we'll set both to R0 (should be an impossible situation)*
  • We do not need to save the CR but need to realign a large stack frame:
    • If two registers are available, we'll set both
    • If only one register is available, we'll set both to that one (should be an impossible situation)*
    • If no registers are available, we'll set both to R0 (should be an impossible situation)*
  • We do not need to save the CR or realign a large stack frame:
    • If R0 is available, we'll set the first scratch register to R0 and the second one will default to R12
    • If two registers are available (other than R0), we'll set both
    • If one register is available, we'll set both to that one
    • If no register is available, we'll set both to R0 (should be an impossible situation)*

*The reason this situation should be impossible is if we have queried the basic block for the right number of available scratch registers, it would have returned true only if they are available. The only way we can get into this situation is if we are trying to insert the prologue/epilogue into a basic block that we have not queried with canUseAsPrologue/canUseAsEpilogue.

[nemanjai]

Finally cleaned up the logic in findScratchRegister. Now it will try its best to find two unique registers. If it can, it sets both (if the pointers are passes) and returns true. If it can find only one, it will set both to that and return true if only one is required or false if two are required. Finally, if it can't find any free registers, it will return false and set both to R0 if the pointers are passed.

The condition that determines whether two registers are needed is now in a separate function which is queried both in canUseAsPrologue and emitPrologue.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

622

use unsigned and ++i, not i++.

627

Please use the term unique here. This isn't about getting two registers - it's about getting unique registers.

639

You need to move this check above line 618. If there are not two unique registers, you need to return false and set SR1 and SR2 to the default values. This will return false and set the scratch registers to be the same.

741

please add:
assert(!(SingleScratchReg && twoScratchRegsRequired) && "I asked for two unique registers!);

This should assure that if you asked for two unique registers, you really got two unique registers.

lib/Target/PowerPC/PPCFrameLowering.h

51

... require two unique scratch registers.

60

... were not available and the defaults are being used.

[nemanjai]

lib/Target/PowerPC/PPCFrameLowering.cpp

622

I don't see why this would make any difference for a primitive type, but I will certainly change it.

627

OK.

639

That seems like a very bad idea to me. What the result would be is that we set the SR1 and SR2 to R0 and R12 respectively knowing that these registers are not available. Don't forget that in emitPrologue/emitEpilogue we do not use the return value at all - we just use the function to get the registers that are available.
If we somehow end up in a situation where we are inserting the prologue/epilogue into a block that doesn't have the R0/R12 available, the function will happily go on it's merry way using the two registers.

741

OK, will do. I only didn't add it here because I have what is essentially an equivalent assert on line 905. Do you think this requires another review?

lib/Target/PowerPC/PPCFrameLowering.h

51

OK.

60

Yeah, makes sense. I'll change it.

[hfinkel]

Some additional comments below. As far as I'm concerned, you can commit and we can do additional fixups as followup.

lib/Target/PowerPC/PPCFrameLowering.cpp

593

It seems like, if we only need one scratch register, and R0 is available, then we can also return true here.

622

ints are preferred (because the compiler can know they can't wrap), so we tend to only use unsiged when necessary for some reason (although we're not entirely consistent about it). ++i is preferred because we only use post-increment if there's a reason (because it can be more expensive for non-primitive types, and no one should have to think about it otherwise).

628

Setting this to R0 seems confusing here. First, it is already set to R0 at the beginning of the function, and by definition, R0 is not actually available (right?). It seems better to set it to -1 in this case (or some other invalid value, like 0 (== PPC::NoRegister)) so that we'd catch an accidental use of it later.

[nemanjai]

lib/Target/PowerPC/PPCFrameLowering.cpp

593

The only reason I don't have that check is that we only know if we absolutely have to have two unique registers. However, when we are in a block that needs to save the CR, we don't absolutely have to have them, but if we do, we emit better code.

I guess the overall idea implemented with this function is this:

• We'll try our best to give you two unique registers
• If we can't, we'll give you one if we can
• If you happen to be asking whether we have enough registers available (1 or 2), we'll let you know

628

I really like the idea of setting it to PPC::NoRegister. After all, if we don't have an available scratch register, it is better to fail than to clobber a live register and have a hard to track-down bug.
The only reason this was setting the register to R0 is to replicate existing semantics.

639

Actually, now that Hal suggested PPC::NoRegister, I actually like that idea best. Then, keeping this check here would have the following outcome:

• If no registers are available, we set both to PPC::NoRegister and return false
• If only one register is available, we set both to that and return true if we only need one, false if we need two
• If two registers are available, we set both accordingly and return true

Finally, I recommend that the calls to this function in emitPrologue/emitEpilogue use the return value of this function in an assert (see below).

739

I think the source of a lot of confusion is that we only use the function here as a mutator and don't check its return value. We ultimately should never be in a situation where this function returns false here. So I recommend having it in an assert:

assert(findScratchRegister(&MBB, false, twoScratchRegsRequired(&MBB), &ScratchReg,
       &TempReg) && "Not enough available scratch registers in this block");

739

Actually, I've come to realize that this assert is redundant since findScratchRegister never currently sets the scratch register to zero.

[nemanjai]

I think Kit mentioned that he still has some concerns about the semantics in the patch. In the meantime and in the interest of getting this patch finalized as soon as possible so that it stops blocking 3.8, I'm uploading an update that I feel addresses Hal's and Kit's comments.

The major change is that we don't set the output parameters to registers that aren't available. If any required register isn't available, we'll set the corresponding output parameter to PPC::NoRegister. This will ensure that we never try to use live registers as scratch registers. Also, to eliminate confusion about what the purpose of findScratchRegister is used for at any particular call, the return value is now always used. When we're emitting the prologue/epilogue, we use it to assert that we have enough registers.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

593

Yes, I agree.

628

R0 is always available because it is the default scratch register. What these checks are essentially doing is determining if we have enough available scratch registers or whether we need to fall back onto the default registers recommended by the ABI.

It's necessary to put the check from line 632 above these checks.
Once that is done, we can simplify this by just using the bit vector iterators to pull the first one or two free registers from the bit vector.

639

This scenario can never happen if this function is used properly.
As long as it is called exactly the same way (same MBB, same conditions for unique registers) it will return false the first time.
In canUseAsPrologue, if this returns false, then the specified block cannot be used for the prologue. If you still try to put the prologue there, then you will get incorrect results, but you have not used the functions properly. If you really want to test for this, check the return value for the call emitPrologue/emitEpilogue, and put an assert on that. It should *always* return true from those functions.

741

OK, yes, that's true. That assert will provide the same coverage.
Yes, this still requires another review because there are still problems in the findScratchRegisters method.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

628

I don't think we can use PPC::NoRegister here - it breaks the contract of the function.
Read the comments in the function carefully - it cleary states what is expected if all registers are not available. We return false and return the default registers (R0 and R12).

I think it's a much better idea to always check the return code, and assert in cases where it is expected to return true. That is, when we are asking whether there are registers available, the return value is used to make a decision (i.e., in canUseAsPrologue). Once the decision is made, we use the same function to get the actual registers available (i.e., emitPrologue). In that case, this function should always return true, or something went wrong.

[kbarton]

lib/Target/PowerPC/PPCFrameLowering.cpp

628

OK, I've thought about this some more and I agree we should be able to use PPC::NoRegister in cases where the function returns false. It's a good idea to ensure everything is being used correctly.

Please make sure to change the documentation of the function, as this is now different from what it originally states. I would also add this (or something similar) to the documentation, to indicate the high-level view of what the function does:

1. If MBB is an entry or exit block, set SR1 and SR2 to R0 and R12 respectively (defaults recommended by the ABI) and return true
2. If MBB is not an entry block, initialize the register scavenger and look for available registers. If TwoUniqueRegsRequired is set to true, it looks for two unique registers. Otherwise, look for a single available register. If the required registers are found, set SR1 and SR2 and return true. If the required registers are not found, set SR1 and SR2 to PPC::NoRegister and return false.

Note that if both SR1 and SR2 are valid parameters, and TwoUniqueRegsRequired is not set, this function will attempt to find two different registers, but still return true if only one register is available (and set SR1 == SR2).

[hfinkel]

I think we've converged; please update the comment as Kit has suggested, and commit this.

lib/Target/PowerPC/PPCFrameLowering.cpp

593

Okay, a comment here to that effect would be useful.

[nemanjai]

This was committed in a series of patches due to bugs I introduced when committing it. This should be the final commit (hopefully no more bugs):
Committed revision 261546.

lib/Target/PowerPC/PPCFrameLowering.h

class PPCFrameLowering: public TargetFrameLowering {
    * then choose a different register.
    *
    * This method will return true if an available register was found (including
    * R0). If no available registers are found, the method returns false and sets
    * ScratchRegister to R0, as per the recommendation in the ABI.
    *
    * \param[in] MBB The machine basic block to find an available register for
    * \param[in] UseAtEnd Specify whether the scratch register will be used at
    *                     the end of the basic block (i.e., will the scratch
    *                     register kill a register defined in the basic block)
    * \param[out] ScratchRegister The scratch register to use
    * \return true if a scratch register was found. false of a scratch register
    *         was not found and R0 is being used as the default.
    */
   bool findScratchRegister(MachineBasicBlock *MBB,
                            bool UseAtEnd,
-                           unsigned *ScratchRegister) const;
+                           unsigned *ScratchRegister,
+                           unsigned *ScratchRegisterCR,
+                           bool Needs2ScratchRegs) const;
 
 public:
   PPCFrameLowering(const PPCSubtarget &STI);
 
   unsigned determineFrameLayout(MachineFunction &MF,
                                 bool UpdateMF = true,
                                 bool UseEstimate = false) const;
 
   /// emitProlog/emitEpilog - These methods insert prolog and epilog code into
   /// the function.
   void emitPrologue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
   void emitEpilogue(MachineFunction &MF, MachineBasicBlock &MBB) const override;

lib/Target/PowerPC/PPCFrameLowering.cpp

for (MachineBasicBlock::iterator MBBI = BI->end(); MBBI != BI->begin(); ) {
           MO.setReg(BP8Reg);
           break;
 
         }
       }
     }
 }
 
-bool PPCFrameLowering::findScratchRegister(MachineBasicBlock *MBB,
-                                           bool UseAtEnd,
-                                           unsigned *ScratchRegister) const {
+bool
+PPCFrameLowering::findScratchRegister(MachineBasicBlock *MBB,
+                                      unsigned *ScratchRegister,
+                                      unsigned *ScratchRegisterCR,
+                                      bool Needs2ScratchRegs) const {
   RegScavenger RS;
   unsigned     R0 = Subtarget.isPPC64() ? PPC::X0 : PPC::R0;
 
   if (ScratchRegister)
     *ScratchRegister = R0;
 
   // If MBB is an entry or exit block, use R0 as the scratch register
+  // and don't change the CR spill register.
   if ((UseAtEnd && MBB->isReturnBlock()) ||
       (!UseAtEnd && (&MBB->getParent()->front() == MBB)))
     return true;
 
   RS.enterBasicBlock(MBB);
 
   if (UseAtEnd && !MBB->empty()) {
-    // The scratch register will be used at the end of the block, so must consider
-    // all registers used within the block
+    // The scratch register will be used at the end of the block, so must
+    // consider all registers used within the block
 
     MachineBasicBlock::iterator MBBI = MBB->getFirstTerminator();
     // If no terminator, back iterator up to previous instruction.
     if (MBBI == MBB->end())
       MBBI = std::prev(MBBI);
 
     if (MBBI != MBB->begin())
       RS.forward(MBBI);
   }
-  
-  if (!RS.isRegUsed(R0)) 
+  // If R0 is available and we don't need another one, we're all good.
+  if (!RS.isRegUsed(R0) && !Needs2ScratchRegs)
     return true;
 
-  unsigned Reg = RS.FindUnusedReg(Subtarget.isPPC64() ? &PPC::G8RCRegClass
-                                  : &PPC::GPRCRegClass);
-  
+  // If we need the second one, initially set it to R0 as well.
+  if (ScratchRegisterCR) {
+    assert(ScratchRegister &&
+           "Looking for the second scratch register without the first?");
+    *ScratchRegisterCR = R0;
+  }
+
+  // Get all the available registers.
+  BitVector BV = RS.getRegsAvailable(Subtarget.isPPC64() ? &PPC::G8RCRegClass :
+                                     &PPC::GPRCRegClass);
   // Make sure the register scavenger was able to find an available register
   // If not, use R0 but return false to indicate no register was available and
   // R0 must be used (as recommended by the ABI)
-  if (Reg == 0)
+  if (BV.empty() || (BV.count() < 2 && Needs2ScratchRegs))
     return false;
 
   if (ScratchRegister)
-    *ScratchRegister = Reg;
+    *ScratchRegister = BV.find_first();
+
+  if (ScratchRegisterCR)
+    *ScratchRegisterCR = BV.find_next(*ScratchRegister);
 
   return true;
 }
 
 bool PPCFrameLowering::canUseAsPrologue(const MachineBasicBlock &MBB) const {
   MachineBasicBlock *TmpMBB = const_cast<MachineBasicBlock *>(&MBB);
 
-  return findScratchRegister(TmpMBB, false, nullptr);
+  /*  Need to determine if this basic block is part of a function that will
+      require a pair of temporary registers in ordert to emit the prologue.
+  */
+  const PPCRegisterInfo *RegInfo =
+      static_cast<const PPCRegisterInfo *>(Subtarget.getRegisterInfo());
+  MachineFunction &MF = *(TmpMBB->getParent());
+  bool HasBP = RegInfo->hasBasePointer(MF);
+  unsigned FrameSize = determineFrameLayout(MF);
+  bool isLargeFrame = !isInt<16>(-FrameSize);
+  MachineFrameInfo *MFI = MF.getFrameInfo();
+  unsigned MaxAlign = MFI->getMaxAlignment();
+
+  bool Needs2ScratchRegs = isLargeFrame && HasBP && MaxAlign > 1;
+
+  return findScratchRegister(TmpMBB, false, nullptr, nullptr,
+                             Needs2ScratchRegs);
 }
 
 bool PPCFrameLowering::canUseAsEpilogue(const MachineBasicBlock &MBB) const {
   MachineBasicBlock *TmpMBB = const_cast<MachineBasicBlock *>(&MBB);
 
-  return findScratchRegister(TmpMBB, true, nullptr);
+  return findScratchRegister(TmpMBB, true, nullptr, nullptr,
+                             /*Needs2ScratchRegs*/ false);
 }
 
 void PPCFrameLowering::emitPrologue(MachineFunction &MF,
                                     MachineBasicBlock &MBB) const {
   MachineBasicBlock::iterator MBBI = MBB.begin();
   MachineFrameInfo *MFI = MF.getFrameInfo();
   const PPCInstrInfo &TII =
       *static_cast<const PPCInstrInfo *>(Subtarget.getInstrInfo());
void PPCFrameLowering::emitPrologue(MachineFunction &MF,
 
   if (MFI->isFrameAddressTaken())
     replaceFPWithRealFP(MF);
 
   // Check if the link register (LR) must be saved.
   PPCFunctionInfo *FI = MF.getInfo<PPCFunctionInfo>();
   bool MustSaveLR = FI->mustSaveLR();
   const SmallVectorImpl<unsigned> &MustSaveCRs = FI->getMustSaveCRs();
+  bool MustSaveCR = !MustSaveCRs.empty();
   // Do we have a frame pointer and/or base pointer for this function?
   bool HasFP = hasFP(MF);
   bool HasBP = RegInfo->hasBasePointer(MF);
 
   unsigned SPReg       = isPPC64 ? PPC::X1  : PPC::R1;
   unsigned BPReg       = RegInfo->getBaseRegister(MF);
   unsigned FPReg       = isPPC64 ? PPC::X31 : PPC::R31;
   unsigned LRReg       = isPPC64 ? PPC::LR8 : PPC::LR;
void PPCFrameLowering::emitPrologue(MachineFunction &MF,
 
   // Regarding this assert: Even though LR is saved in the caller's frame (i.e.,
   // LROffset is positive), that slot is callee-owned. Because PPC32 SVR4 has no
   // Red Zone, an asynchronous event (a form of "callee") could claim a frame &
   // overwrite it, so PPC32 SVR4 must claim at least a minimal frame to save LR.
   assert((isPPC64 || !isSVR4ABI || !(!FrameSize && (MustSaveLR || HasFP))) &&
          "FrameSize must be >0 to save/restore the FP or LR for 32-bit SVR4.");
 
-  findScratchRegister(&MBB, false, &ScratchReg);
+  findScratchRegister(&MBB, false, &ScratchReg, &TempReg, MustSaveCR);
+
   assert(ScratchReg && "No scratch register!");
-         
+  bool SingleScratchReg = ScratchReg == TempReg;
   int LROffset = getReturnSaveOffset();
 
   int FPOffset = 0;
   if (HasFP) {
     if (isSVR4ABI) {
       MachineFrameInfo *FFI = MF.getFrameInfo();
       int FPIndex = FI->getFramePointerSaveIndex();
       assert(FPIndex && "No Frame Pointer Save Slot!");
void PPCFrameLowering::emitPrologue(MachineFunction &MF,
   if (HasBP && MaxAlign > 1)
     assert(isPowerOf2_32(MaxAlign) && isInt<16>(MaxAlign) &&
            "Invalid alignment!");
 
   // Frames of 32KB & larger require special handling because they cannot be
   // indexed into with a simple STDU/STWU/STD/STW immediate offset operand.
   bool isLargeFrame = !isInt<16>(NegFrameSize);
 
+  assert((isPPC64 || !MustSaveCR) &&
+         "Prologue CR saving supported only in 64-bit mode");
+
+  // If we need to spill the CR and the LR but we don't have two separate
+  // registers available, we must spill them one at a time
+  if (MustSaveCR && SingleScratchReg && MustSaveLR) {
+    // FIXME: In the ELFv2 ABI, we are not required to save all CR fields.
+    // If only one or two CR fields are clobbered, it could be more
+    // efficient to use mfocrf to selectively save just those fields.
+    MachineInstrBuilder MIB =
+      BuildMI(MBB, MBBI, dl, TII.get(PPC::MFCR8), TempReg);
+    for (unsigned i = 0, e = MustSaveCRs.size(); i != e; ++i)
+      MIB.addReg(MustSaveCRs[i], RegState::ImplicitKill);
+    BuildMI(MBB, MBBI, dl, TII.get(PPC::STW8))
+      .addReg(TempReg, getKillRegState(true))
+      .addImm(8)
+      .addReg(SPReg);
+  }
+
   if (MustSaveLR)
     BuildMI(MBB, MBBI, dl, MFLRInst, ScratchReg);
 
-  assert((isPPC64 || MustSaveCRs.empty()) &&
-         "Prologue CR saving supported only in 64-bit mode");
-
-  if (!MustSaveCRs.empty()) { // will only occur for PPC64
+  if (MustSaveCR &&
+      !(SingleScratchReg && MustSaveLR)) { // will only occur for PPC64
     // FIXME: In the ELFv2 ABI, we are not required to save all CR fields.
     // If only one or two CR fields are clobbered, it could be more
     // efficient to use mfocrf to selectively save just those fields.
     MachineInstrBuilder MIB =
       BuildMI(MBB, MBBI, dl, TII.get(PPC::MFCR8), TempReg);
     for (unsigned i = 0, e = MustSaveCRs.size(); i != e; ++i)
       MIB.addReg(MustSaveCRs[i], RegState::ImplicitKill);
   }
void PPCFrameLowering::emitPrologue(MachineFunction &MF,
 
   if (MustSaveLR)
     // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
     BuildMI(MBB, MBBI, dl, StoreInst)
       .addReg(ScratchReg)
       .addImm(LROffset)
       .addReg(SPReg);
 
-  if (!MustSaveCRs.empty()) // will only occur for PPC64
+  if (MustSaveCR &&
+      !(SingleScratchReg && MustSaveLR)) // will only occur for PPC64
     BuildMI(MBB, MBBI, dl, TII.get(PPC::STW8))
       .addReg(TempReg, getKillRegState(true))
       .addImm(8)
       .addReg(SPReg);
 
   // Skip the rest if this is a leaf function & all spills fit in the Red Zone.
   if (!FrameSize) return;
 
   // Adjust stack pointer: r1 += NegFrameSize.
   // If there is a preferred stack alignment, align R1 now
 
   if (HasBP) {
     // Save a copy of r1 as the base pointer.
     BuildMI(MBB, MBBI, dl, OrInst, BPReg)
       .addReg(SPReg)
       .addReg(SPReg);
   }
 
   if (HasBP && MaxAlign > 1) {
     if (isPPC64)
       BuildMI(MBB, MBBI, dl, TII.get(PPC::RLDICL), ScratchReg)
         .addReg(SPReg)
         .addImm(0)
         .addImm(64 - Log2_32(MaxAlign));
     else // PPC32...
       BuildMI(MBB, MBBI, dl, TII.get(PPC::RLWINM), ScratchReg)
         .addReg(SPReg)
         .addImm(0)
         .addImm(32 - Log2_32(MaxAlign))
         .addImm(31);
     if (!isLargeFrame) {
       BuildMI(MBB, MBBI, dl, SubtractImmCarryingInst, ScratchReg)
         .addReg(ScratchReg, RegState::Kill)
         .addImm(NegFrameSize);
     } else {
+      assert(!SingleScratchReg && "Only a single scratch reg available");
       BuildMI(MBB, MBBI, dl, LoadImmShiftedInst, TempReg)
         .addImm(NegFrameSize >> 16);
       BuildMI(MBB, MBBI, dl, OrImmInst, TempReg)
         .addReg(TempReg, RegState::Kill)
         .addImm(NegFrameSize & 0xFFFF);
       BuildMI(MBB, MBBI, dl, SubtractCarryingInst, ScratchReg)
         .addReg(ScratchReg, RegState::Kill)
         .addReg(TempReg, RegState::Kill);
for (unsigned I = 0, E = CSI.size(); I != E; ++I) {
       // This is a bit of a hack: CR2LT, CR2GT, CR2EQ and CR2UN are just
       // subregisters of CR2. We just need to emit a move of CR2.
       if (PPC::CRBITRCRegClass.contains(Reg))
         continue;
 
       // For SVR4, don't emit a move for the CR spill slot if we haven't
       // spilled CRs.
       if (isSVR4ABI && (PPC::CR2 <= Reg && Reg <= PPC::CR4)
-          && MustSaveCRs.empty())
+          && !MustSaveCR)
         continue;
 
       // For 64-bit SVR4 when we have spilled CRs, the spill location
       // is SP+8, not a frame-relative slot.
       if (isSVR4ABI && isPPC64 && (PPC::CR2 <= Reg && Reg <= PPC::CR4)) {
         // In the ELFv1 ABI, only CR2 is noted in CFI and stands in for
         // the whole CR word.  In the ELFv2 ABI, every CR that was
         // actually saved gets its own CFI record.
void PPCFrameLowering::emitEpilogue(MachineFunction &MF,
   bool isPPC64 = Subtarget.isPPC64();
   // Get the ABI.
   bool isSVR4ABI = Subtarget.isSVR4ABI();
 
   // Check if the link register (LR) has been saved.
   PPCFunctionInfo *FI = MF.getInfo<PPCFunctionInfo>();
   bool MustSaveLR = FI->mustSaveLR();
   const SmallVectorImpl<unsigned> &MustSaveCRs = FI->getMustSaveCRs();
+  bool MustSaveCR = !MustSaveCRs.empty();
   // Do we have a frame pointer and/or base pointer for this function?
   bool HasFP = hasFP(MF);
   bool HasBP = RegInfo->hasBasePointer(MF);
 
   unsigned SPReg      = isPPC64 ? PPC::X1  : PPC::R1;
   unsigned BPReg      = RegInfo->getBaseRegister(MF);
   unsigned FPReg      = isPPC64 ? PPC::X31 : PPC::R31;
   unsigned ScratchReg = 0;
   unsigned TempReg     = isPPC64 ? PPC::X12 : PPC::R12; // another scratch reg
   const MCInstrDesc& MTLRInst = TII.get( isPPC64 ? PPC::MTLR8
                                                  : PPC::MTLR );
   const MCInstrDesc& LoadInst = TII.get( isPPC64 ? PPC::LD
                                                  : PPC::LWZ );
   const MCInstrDesc& LoadImmShiftedInst = TII.get( isPPC64 ? PPC::LIS8
                                                            : PPC::LIS );
   const MCInstrDesc& OrImmInst = TII.get( isPPC64 ? PPC::ORI8
                                                   : PPC::ORI );
   const MCInstrDesc& AddImmInst = TII.get( isPPC64 ? PPC::ADDI8
                                                    : PPC::ADDI );
   const MCInstrDesc& AddInst = TII.get( isPPC64 ? PPC::ADD8
                                                 : PPC::ADD4 );
-  
   int LROffset = getReturnSaveOffset();
 
   int FPOffset = 0;
 
-  findScratchRegister(&MBB, true, &ScratchReg);
+  findScratchRegister(&MBB, true, &ScratchReg, &TempReg, MustSaveCR);
+
   assert(ScratchReg && "No scratch register!");
-  
+  bool SingleScratchReg = ScratchReg == TempReg;
   if (HasFP) {
     if (isSVR4ABI) {
       MachineFrameInfo *FFI = MF.getFrameInfo();
       int FPIndex = FI->getFramePointerSaveIndex();
       assert(FPIndex && "No Frame Pointer Save Slot!");
       FPOffset = FFI->getObjectOffset(FPIndex);
     } else {
       FPOffset = getFramePointerSaveOffset();
if (FI->hasFastCall()) {
         .addImm(FrameSize);
     } else {
       BuildMI(MBB, MBBI, dl, LoadInst, SPReg)
         .addImm(0)
         .addReg(SPReg);
     }
   }
 
+  assert((isPPC64 || !MustSaveCR) &&
+         "Epilogue CR restoring supported only in 64-bit mode");
+
+  // If we need to save both the LR and the CR and we only have one available
+  // scratch register, we must do them one at a time.
+  if (MustSaveCR && SingleScratchReg && MustSaveLR) {
+    BuildMI(MBB, MBBI, dl, TII.get(PPC::LWZ8), TempReg)
+      .addImm(8)
+      .addReg(SPReg);
+    for (unsigned i = 0, e = MustSaveCRs.size(); i != e; ++i)
+      BuildMI(MBB, MBBI, dl, TII.get(PPC::MTOCRF8), MustSaveCRs[i])
+        .addReg(TempReg, getKillRegState(i == e-1));
+  }
+
   if (MustSaveLR)
     BuildMI(MBB, MBBI, dl, LoadInst, ScratchReg)
       .addImm(LROffset)
       .addReg(SPReg);
 
-  assert((isPPC64 || MustSaveCRs.empty()) &&
-         "Epilogue CR restoring supported only in 64-bit mode");
-
-  if (!MustSaveCRs.empty()) // will only occur for PPC64
+  if (MustSaveCR &&
+      !(SingleScratchReg && MustSaveLR)) // will only occur for PPC64
     BuildMI(MBB, MBBI, dl, TII.get(PPC::LWZ8), TempReg)
       .addImm(8)
       .addReg(SPReg);
 
   if (HasFP)
     BuildMI(MBB, MBBI, dl, LoadInst, FPReg)
       .addImm(FPOffset)
       .addReg(SPReg);
 
   if (FI->usesPICBase())
     // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
     BuildMI(MBB, MBBI, dl, LoadInst)
       .addReg(PPC::R30)
       .addImm(PBPOffset)
       .addReg(SPReg);
 
   if (HasBP)
     BuildMI(MBB, MBBI, dl, LoadInst, BPReg)
       .addImm(BPOffset)
       .addReg(SPReg);
 
-  if (!MustSaveCRs.empty()) // will only occur for PPC64
+  if (MustSaveCR &&
+      !(SingleScratchReg && MustSaveLR)) // will only occur for PPC64
     for (unsigned i = 0, e = MustSaveCRs.size(); i != e; ++i)
       BuildMI(MBB, MBBI, dl, TII.get(PPC::MTOCRF8), MustSaveCRs[i])
         .addReg(TempReg, getKillRegState(i == e-1));
 
   if (MustSaveLR)
     BuildMI(MBB, MBBI, dl, MTLRInst).addReg(ScratchReg);
 
   // Callee pop calling convention. Pop parameter/linkage area. Used for tail