[PATCH] D17294: Fix for PR 26500

[Kit Barton via llvm-commits]

kbarton added inline comments.

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:621
@@ +620,3 @@
+    int FirstScratchReg = BV.find_first();
+    *SR1 = FirstScratchReg == -1 ? (unsigned)PPC::NoRegister : FirstScratchReg;
+  }
----------------
kbarton wrote:
> 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.
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).


Repository:
  rL LLVM

http://reviews.llvm.org/D17294