[PATCH] D17294: Fix for PR 26500

[Kit Barton via llvm-commits]

kbarton added inline comments.

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:601
@@ +600,3 @@
+  // If we need the second one, initially set it to R0 as well.
+  if (ScratchRegisterCR) {
+    assert(ScratchRegister &&
----------------
You don't need to do this now, since you've initialized ScratchRegisterCR at the beginning.


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:616
@@ +615,3 @@
+
+  // Don't consider callee-saved registers
+  for (int i = 0; CSRegs[i]; i++)
----------------
Probably should expand on this comment a bit...

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:655
@@ +654,3 @@
+  // and the stack frame is large, we need two scratch registers.
+  bool Needs2ScratchRegs = IsLargeFrame && HasBP && MaxAlign > 1;
+
----------------
You should refactor this logic into a separate method and use it when calling findScratchRegister in emitPrologue and emitEpilogue, as well as here. 

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:661
@@ -612,3 +660,3 @@
 bool PPCFrameLowering::canUseAsEpilogue(const MachineBasicBlock &MBB) const {
   MachineBasicBlock *TmpMBB = const_cast<MachineBasicBlock *>(&MBB);
 
----------------
Need to update this call to findScratchRegister as well, since the epilogue can use two scratch registers. 

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:753
@@ -703,2 +752,3 @@
 
-  findScratchRegister(&MBB, false, &ScratchReg);
+  findScratchRegister(&MBB, false, MustSaveCR, &ScratchReg, &TempReg);
+
----------------
Need to use the same logic when calling findScratchRegister here, as used above in canUseAsPrologue.

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:883
@@ -813,2 +882,3 @@
 
+  // This condition must be kept in sync with canUseAsPrologue.
   if (HasBP && MaxAlign > 1) {
----------------
This is currently not in sync, since you're only using MustSaveCRs to indicate whether you need two scratch registers. 

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:1106
@@ -1033,2 +1105,3 @@
 
-  findScratchRegister(&MBB, true, &ScratchReg);
+  findScratchRegister(&MBB, true, MustSaveCR, &ScratchReg, &TempReg);
+
----------------
We need to make a corresponding change to findScratchRegister in canUseAsEpilogue.

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.h:46
@@ -44,1 +45,3 @@
+   * to be realigned) the function will check for and set two registers.
+   *
    * \param[in] MBB The machine basic block to find an available register for
----------------
What happens if it cannot find two registers?


Repository:
  rL LLVM

http://reviews.llvm.org/D17294