This patch looks good to me now.  :-)<br><br>p.s. For the second item, I wish to have the unit test on the mangled<br>name of the functions with va_list as the argument.  Which is<br>test/CodeGenCXX/mangle-valist.cpp exactly.  Thanks.<br>
<br>Sincerely,<br>Logan<br><br><div class="gmail_quote">On Thu, Oct 4, 2012 at 1:36 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 Logan,<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">Very appreciate for your review.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I addressed the first item: </span>Keep the void* test in builtins-arm.c for apcs-gnu ABI<u></u><u></u></p>
<p class="MsoNormal">Please review the attached patch.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">For adding the name mangling test case, I already included one (test/CodeGenCXX/mangle-valist.cpp) in my previous patch. Do you mean something else?<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks a lot,<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>
<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>
<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"><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""> Logan Chien [mailto:<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>] <br>
<b>Sent:</b> Tuesday, October 02, 2012 10:34 PM<br><b>To:</b> Weiming Zhao<br><b>Cc:</b> Jordan Rose; <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><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal" style="margin-bottom:12.0pt">LGTM.</p><div><div class="h5"><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></div></div><p></p><div><div class="h5"><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></div></div></div></div></blockquote></div><br>