LGTM.  Any further comments?<br><br>BTW, Weiming, do you have the commit access?<br><br>Logan<br><br><div class="gmail_quote">On Sat, Oct 6, 2012 at 1:10 AM, Weiming Zhao <span dir="ltr"><<a href="mailto:weimingz@codeaurora.org" target="_blank">weimingz@codeaurora.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div link="blue" vlink="purple" lang="EN-US"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Jordan,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks for reviewing.<u></u><u></u></span></p>
<p><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><span>1.<span style="font:7.0pt "Times New Roman"">       </span></span></span><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sema.h is removed.<u></u><u></u></span></p>
<p><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><span>2.<span style="font:7.0pt "Times New Roman"">       </span></span></span><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I glad to find that we don’t need the workaround for the exception. Someone fixed something in clang. So we don’t need to relax the assertion.<u></u><u></u></span></p>
<p><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><span>3.<span style="font:7.0pt "Times New Roman"">       </span></span></span><u></u><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Fixed. To be consistent with “IsThumb”, I name the variable  “IsAAPCSS”. But I can’t completely decide it as constructor because setABI() will change the ABI. For consistency, the variable name.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Please help to review the attached patch.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Weiming<u></u><u></u></span></p><div class="im"><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
</div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p></div><div><div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Jordan Rose [mailto:<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>] <br>
<b>Sent:</b> Thursday, October 04, 2012 6:19 PM<br><b>To:</b> Logan Chien<br><b>Cc:</b> Weiming Zhao; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a></span></p><div class="im"><br><b>Subject:</b> Re: [cfe-commits] Bug 11709 Fix: va_list on ARM is not following AAPCS 7.1.4<u></u><u></u></div>
<p></p></div></div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">A couple comments (sorry for the long long delay):<u></u><u></u></p></div><div><div class="h5"><div><p class="MsoNormal"><u></u> <u></u></p>
</div><div><p class="MsoNormal">- AST can't depend on Sema. Is there a reason for the include of Sema.h in ASTContext.cpp?<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">
- It would be a good idea to leave a comment in ExprClassification.cpp about why __va_list needs an exception.<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">- For ARMTargetInfo::getBuiltinVaListKind, maybe you can just decide this at construction time and stick it in a local variable? 'unsigned usesAAPCS : 1'?<u></u><u></u></p>
</div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Thanks for working on this, Weiming, Logan.<u></u><u></u></p></div><div><p class="MsoNormal">Jordan<u></u><u></u></p></div><div><p class="MsoNormal">
<u></u> <u></u></p></div><p class="MsoNormal"><u></u> <u></u></p><div><div><p class="MsoNormal">On Oct 2, 2012, at 22:34 , Logan Chien <<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>> wrote:<u></u><u></u></p>
</div><p class="MsoNormal"><br><br><u></u><u></u></p><p class="MsoNormal" style="margin-bottom:12.0pt">LGTM.<br><br>However, I think we can enhance the test cases a little.<br>1. Keep the void* test in builtins-arm.c for apcs-gnu ABI.<br>
2. Add the unit test for va_list name mangling.<br><br>Sincerely,<br>Logan<br><br>ps. The attached patch is the revised patch with the updated<br>test cases.<u></u><u></u></p><div><p class="MsoNormal">On Thu, Sep 20, 2012 at 6:13 AM, Weiming Zhao <<a href="mailto:weimingz@codeaurora.org" target="_blank">weimingz@codeaurora.org</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">Ping...<br><br>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by<br>The Linux Foundation<u></u><u></u></p></div><p class="MsoNormal"><mangle-valist.cpp><builtins-arm.c><0001-Bug-11709-Change-the-definition-of-va_list-to-meet-A.patch><u></u><u></u></p>
</div><p class="MsoNormal"><u></u> <u></u></p></div></div></div></div></blockquote></div><br>