Hey Jakob,<div><br></div><div>Thanks for the reviews:<br><br></div><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; color: rgb(34, 34, 34); ">>>I was just about to remove RegScavenger::backward(). Will you be<br>
>>needing it?</span></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><br></span></font></div><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; color: rgb(34, 34, 34); ">No, go ahead and kill it (if nobody else needs it).<br>
</span><div class="gmail_quote"><br></div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; color: rgb(34, 34, 34); ">>>Perhaps CalleeSavedRegs could be renamed to UsedCalleeSavedRegs or<br>
>>something along those lines? Just so it is clear it doesn't correspond<br></span><div class="gmail_quote"><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; color: rgb(34, 34, 34); ">>>to TRI->getCalleeSavedRegs().</span></div>
<div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><br></span></font></div><div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;">I just used what was there. You're right, UsedCalleeSavedRegs is what I it should be. I'll change the name.</span></font></div>
<div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><br></span></font></div><div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><span class="Apple-style-span" style="font-size: 13px; ">>>Please check the assert condition - it doesn't do what the comment says.</span></span></font></div>
<div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><br></span></font></div><div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><span class="Apple-style-span" style="font-size: 13px; ">Thanks, I'll fix it.<br>
<br>>>Have you considered simply marking the free CSRs as used in<br>>>initRegState? Then you wouldn't have to disable check for these registers.</span></span></font></div><div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;"><br>
</span></font></div><div class="gmail_quote"><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse: collapse;">If a CSR isn't used in a function, it doesn't need to be spilled on entry/restored on return, so it should be usable by RS, hence must not be taken out of RegsAvailable. Perhaps I'm not following you here(?) </span></font></div>
<div class="gmail_quote"><br></div><div class="gmail_quote"><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; color: rgb(34, 34, 34); "></span>On Thu, Aug 6, 2009 at 12:42 PM, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im"><br>
On 06/08/2009, at 19.23, Jakob Stoklund Olesen wrote:<br>
>> void RegScavenger::enterBasicBlock(MachineBasicBlock *mbb) {<br>
>> MachineFunction &MF = *mbb->getParent();<br>
>> const TargetMachine &TM = MF.getTarget();<br>
>> @@ -94,6 +117,7 @@<br>
>> assert((NumPhysRegs == 0 || NumPhysRegs == TRI->getNumRegs()) &&<br>
>> "Target changed?");<br>
>><br>
>> + // Self-initialize.<br>
>> if (!MBB) {<br>
>> NumPhysRegs = TRI->getNumRegs();<br>
>> RegsAvailable.resize(NumPhysRegs);<br>
>> @@ -104,29 +128,23 @@<br>
>> // Create callee-saved registers bitvector.<br>
>> CalleeSavedRegs.resize(NumPhysRegs);<br>
>> const unsigned *CSRegs = TRI->getCalleeSavedRegs();<br>
>> - if (CSRegs != NULL)<br>
>> - for (unsigned i = 0; CSRegs[i]; ++i)<br>
>> - CalleeSavedRegs.set(CSRegs[i]);<br>
>> - }<br>
><br>
> Perhaps CalleeSavedRegs could be renamed to UsedCalleeSavedRegs or<br>
> something along those lines? Just so it is clear it doesn't correspond<br>
> to TRI->getCalleeSavedRegs().<br>
<br>
</div>One more thing: FindUnusedReg takes an ExCalleeSaved parameter. When<br>
that is set, all the non-spilled CSRs must be excluded. Otherwise a<br>
non-spilled CSR could be used without being spilled first.</blockquote><div><br></div><div>If a CSR isn't used, it doesn't need to be spilled, yadda yadda yadda, RS can use it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br> From ARMLoadStoreOptimizer.cpp:<br>
<br>
// Find a scratch register. Make sure it's a call clobbered<br>
register or<br>
// a spilled callee-saved register.<br>
unsigned Scratch = RS->FindUnusedReg(&ARM::GPRRegClass, true);<br>
<br>
I think what you want is a FreeCalleeSavedRegs variable.</blockquote><div><br></div><div>How about UsedCalleeSavedRegs?</div><div><br></div><div>Thanks much!</div><div><br></div><div>John</div><div><br></div><div><br></div>
</div></div>