Hi Jordan and Logan, Thanks for reviewing. I don't have commit access. Can you help to commit it?
Thanks, Weiming Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -----Original Message----- From: Jordan Rose [mailto:[email protected]] Sent: Monday, October 08, 2012 7:14 PM To: Logan Chien Cc: [email protected]; [email protected] Subject: Re: [cfe-commits] Bug 11709 Fix: va_list on ARM is not following AAPCS 7.1.4 LGTM as well. On Oct 8, 2012, at 19:02 , Logan Chien <[email protected]> wrote: > LGTM. Any further comments? > > BTW, Weiming, do you have the commit access? > > Logan > > On Sat, Oct 6, 2012 at 1:10 AM, Weiming Zhao <[email protected]> wrote: > Hi Jordan, > > > > Thanks for reviewing. > > 1. Sema.h is removed. > > 2. 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. > > 3. 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. > > > > Please help to review the attached patch. > > > > Thanks, > > Weiming > > > > > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > > > From: Jordan Rose [mailto:[email protected]] > Sent: Thursday, October 04, 2012 6:19 PM > To: Logan Chien > Cc: Weiming Zhao; [email protected] > > > Subject: Re: [cfe-commits] Bug 11709 Fix: va_list on ARM is not > following AAPCS 7.1.4 > > > > A couple comments (sorry for the long long delay): > > > > - AST can't depend on Sema. Is there a reason for the include of Sema.h in ASTContext.cpp? > > > > - It would be a good idea to leave a comment in ExprClassification.cpp about why __va_list needs an exception. > > > > - For ARMTargetInfo::getBuiltinVaListKind, maybe you can just decide this at construction time and stick it in a local variable? 'unsigned usesAAPCS : 1'? > > > > Thanks for working on this, Weiming, Logan. > > Jordan > > > > > > On Oct 2, 2012, at 22:34 , Logan Chien <[email protected]> wrote: > > > > > LGTM. > > However, I think we can enhance the test cases a little. > 1. Keep the void* test in builtins-arm.c for apcs-gnu ABI. > 2. Add the unit test for va_list name mangling. > > Sincerely, > Logan > > ps. The attached patch is the revised patch with the updated test > cases. > > On Thu, Sep 20, 2012 at 6:13 AM, Weiming Zhao <[email protected]> wrote: > > Ping... > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > <mangle-valist.cpp><builtins-arm.c><0001-Bug-11709-Change-the-definiti > on-of-va_list-to-meet-A.patch> > > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
