Hi,<div><br></div><div>fastcc uses FastCC_ARM_APCS (not CC_ARM_APCS) if targets don't support AAPCS ABI, and FastCC_ARM_APCS is specific to fastcc, that is, if we merge it into CallingConv::C, there will be extra checks caused by FastCC_ARM_APCS when we handle CallingConv::C, but CallingConv::C doesn't need these checks. In general, we use CallingConv::C much, and fastcc is rare, this will be unnecessary overhead to CallingConv::C, and we just move similar checks from fasctcc to CCC.</div>
<div><br></div><div>For the fast-isel-fastcc.patch attached in this email, I add test cases for {VFP, noVFP} x {fastcc, no fastcc}, no changes for rest.</div><div><br></div><div>Jush</div><div><br><div class="gmail_quote">
On Sat, Aug 11, 2012 at 2:20 AM, Renato Golin <span dir="ltr"><<a href="mailto:rengolin@systemcall.org" target="_blank">rengolin@systemcall.org</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">On 10 August 2012 12:25, Jush Lu <<a href="mailto:jush.msn@gmail.com">jush.msn@gmail.com</a>> wrote:<br>
> Hi all,<br>
><br>
> As Renato's suggestion, I split the last patch into two patches.<br>
<br>
</div>It was actually Chad's suggestion, I just agreed with him... :D<br>
<div class="im"><br>
<br>
> 'fast-isel-fastcc.patch' is for fix, and another is for clean up.<br>
<br>
</div>The clean-up is harmless, and should pose no problems. However, it<br>
seems you ended up with two implementations of the same thing (sorry I<br>
missed that in the first review).<br>
<br>
AFAIK, fastcc in ARM is the same as AAPCS, so there is no point in<br>
having two similar logics for both. The original implementation,<br>
though, seemed to be wrong assuming "fastcc" (like CCC) should be<br>
default to APCS, rather than AAPCS.<br>
<br>
I think the best course of action is to common up the two<br>
implementations (ie. leave the fall through as it is), by adding<br>
yet-another test in the end (and on ABI check), taking (CC ==<br>
CallingConv::Fast) into consideration.<br>
<br>
And add the working version of the tests, for completeness.<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
cheers,<br>
--renato<br>
<br>
<a href="http://systemcall.org/" target="_blank">http://systemcall.org/</a><br>
</div></div></blockquote></div><br></div>