<b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><span style="vertical-align:baseline;white-space:pre-wrap"></span></b><div>
<div style="display:inline!important"><div style="display:inline!important"><b style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important"><b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline">Hi Chad,</span></b></div></span></b></div></span></b></div></span></b></b></b></b></div></div></div><div><br></div><div><b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"></b><div style="display:inline!important">
<b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"></b><div style="display:inline!important"><b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important"><b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b id="internal-source-marker_0.20839887741021812" style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline">In this patch, the first part</span></b></div></span></b></div></span></b></div>
</span></b></b></b></b></div></div></div><div><div style="display:inline!important"><div style="display:inline!important"><b style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important"><b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><div style="display:inline!important">
<b style="font-size:15.199999809265137px;font-weight:normal"><span style="vertical-align:baseline"><br></span></b></div></span></b></div></span></b></div></span></b></b></b></b></div></div></div><div><div style="display:inline!important">
<div style="display:inline!important"><b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><span style="vertical-align:baseline;white-space:pre-wrap">   case CallingConv::Fast:</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">-    // Ignore fastcc. Silence compiler warnings.</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-    (void)RetFastCC_ARM_APCS;</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-    (void)FastCC_ARM_APCS;</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+    if (Subtarget->hasVFP2() && !isVarArg) {</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+      if (!Subtarget->isAAPCS_ABI())</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+        return (Return ? RetFastCC_ARM_APCS : FastCC_ARM_APCS);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+      // For AAPCS ABI targets, just use VFP variant of the calling convention.</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+      return (Return ? RetCC_ARM_AAPCS_VFP : CC_ARM_AAPCS_VFP);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+    }</span><br><span style="vertical-align:baseline;white-space:pre-wrap">     // Fallthrough</span></b></div>
</div></div><div><br></div><span style="vertical-align:baseline;white-space:pre-wrap">This part only adds support for fastcc, and fastcc support will fix the inconsistency of calling convention, there is no extra code for fixing inconsistency of calling convention.</span><span style="vertical-align:baseline"> </span><span style="vertical-align:baseline;white-space:pre-wrap">Should I split this part? it is hard to split it, I am afraid that my bad description leads you to think that the rest of this patch is to fix the inconsistency of calling convention, but it doesn't .</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap"></span><br><span style="vertical-align:baseline;white-space:pre-wrap"></span><br><span style="vertical-align:baseline;white-space:pre-wrap">the rest of this patch</span><div>
<span style="white-space:pre-wrap"><br></span><span style="vertical-align:baseline;white-space:pre-wrap"></span><b id="internal-source-marker_0.20839887741021812" style="font-family:Arial;font-size:15.199999809265137px;text-indent:130.1999969482422px;font-weight:normal"><span style="vertical-align:baseline;white-space:pre-wrap">   case CallingConv::C:</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">     // Use target triple & subtarget features to do actual dispatch.</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-    if (Subtarget->isAAPCS_ABI()) {</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">-      if (Subtarget->hasVFP2() &&</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-          TM.Options.FloatABIType == FloatABI::Hard && !isVarArg)</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">-        return (Return ? RetCC_ARM_AAPCS_VFP: CC_ARM_AAPCS_VFP);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-      else</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-        return (Return ? RetCC_ARM_AAPCS: CC_ARM_AAPCS);</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">-    } else</span><br><span style="vertical-align:baseline;white-space:pre-wrap">-        return (Return ? RetCC_ARM_APCS: CC_ARM_APCS);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+    if (!Subtarget->isAAPCS_ABI())</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+      return (Return ? RetCC_ARM_APCS : CC_ARM_APCS);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+    if (Subtarget->hasVFP2() &&</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+        TM.Options.FloatABIType == FloatABI::Hard && !isVarArg)</span><br><span style="vertical-align:baseline;white-space:pre-wrap">+      return (Return ? RetCC_ARM_AAPCS_VFP: CC_ARM_AAPCS_VFP);</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap">+    return (Return ? RetCC_ARM_AAPCS: CC_ARM_AAPCS);</span><br><span style="vertical-align:baseline;white-space:pre-wrap">   case CallingConv::ARM_AAPCS_VFP:</span></b><br>
<span style="vertical-align:baseline;white-space:pre-wrap"></span><br><span style="vertical-align:baseline;white-space:pre-wrap">This part doesn’t fix anything, it just simplifies if statements, no functionality changes in this part.</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap"></span><span style="vertical-align:baseline;white-space:pre-wrap"></span><span style="vertical-align:baseline;white-space:pre-wrap"></span><br><span style="vertical-align:baseline;white-space:pre-wrap">In the attached patch, I only remove the ‘else’ from ‘else if’ as your comments, the rest is same as the last patch, I didn’t do split yet.</span><br>
<span style="vertical-align:baseline;white-space:pre-wrap"></span><br><span style="vertical-align:baseline;white-space:pre-wrap">Thanks,</span></div><div><span style="white-space:pre-wrap">Jush<br></span><br><div class="gmail_quote">
On Fri, Aug 10, 2012 at 3:07 AM, Chad Rosier <span dir="ltr"><<a href="mailto:mcrosier@apple.com" target="_blank">mcrosier@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>Jush,</div><div>Based on your description this patch has two fixes.  I realize these are very small changes, but can you please split this into two patches?</div><div><br></div><div>
Also</div><div><br></div><div><div>+    if (!Subtarget->isAAPCS_ABI())</div><div>+      return (Return ? RetCC_ARM_APCS : CC_ARM_APCS);</div><div>+    else if (Subtarget->hasVFP2() &&</div><div>+           TM.Options.FloatABIType == FloatABI::Hard && !isVarArg)</div>
<div>+      return (Return ? RetCC_ARM_AAPCS_VFP: CC_ARM_AAPCS_VFP);</div></div><div><br></div><div>No need for the 'else' in the 'else if' since the previous statement was a return.</div><div><br></div><div>
 Chad</div><br><div><div><div class="h5"><div>On Aug 9, 2012, at 4:53 AM, Jush Lu wrote:</div><br></div></div><blockquote type="cite"><b style="font-family:'LiHei Pro';font-size:medium;font-weight:normal"><div><div class="h5">
<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Hi,</span><br>
<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">This patch adds support for fast calling convention, and this patch is almost same as r117119, I just copied a piece of code from CCAssignFnForNode which is in lib/Target/ARM/ARMISelLowering.cpp</span><br>

<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Currently ARMFastISel handles fastcc for caller side by simply falling through to other calling conventions such as CC_ARM_AAPCS, but the callee with fastcc always uses CC_ARM_AAPCS_VFP calling convention if VFP2 is available. This inconsistency of calling conventions will make problems, and this patch will fix it as well.</span><br>

<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Please review this patch, thanks.</span><br>

<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br></div></div><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Jush</span></b>
<span><fast-isel-fastcc.patch></span></blockquote></div><br></div></blockquote></div><br></div>