[cfe-commits] Bug 11709 Fix: va_list on ARM is not following AAPCS 7.1.4

[Weiming Zhao]

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:jordan_rose at apple.com] 
Sent: Thursday, October 04, 2012 6:19 PM
To: Logan Chien
Cc: Weiming Zhao; cfe-commits at cs.uiuc.edu
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 <tzuhsiang.chien at gmail.com> 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 <weimingz at codeaurora.org>
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-definition-of-
va_list-to-meet-A.patch>

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121005/3ef54753/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Fix-PR-11709-Change-the-definition-of-va_list-to-mee.patch
Type: application/octet-stream
Size: 9418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121005/3ef54753/attachment.obj>