[llvm-commits] [ARM] Fix for blocking PR13790. AAPCS byval params issue.

[James Molloy]

Hi Stepan,

A couple of comments:

  * Why the need to change the LDRB_POST_IMM-Crash.ll testcase so
drastically? (changing from doubles to i8s)

  * if (Subtarget->isAAPCS_ABI()  <-- surely this should be enabled when
using the aapcs-vfp ABI too?

Apart from those comments, it LGTM!

Cheers,

James

On Thu, 2012-10-11 at 15:25 +0100, Stepan Dyatkovskiy wrote:
> Hi Anton,
> 
>  > How will you patch influence APCS?
> 
> To make all clear, I updated patch and made it fixing AAPCS case only.
> 
> I can't get codegen to emit 8 bytes aligned structure parameter, and I 
> can't get to use 8-bytes aligned stack. ARM target ignores 
> --stack-alignment option.
> 
> But if some people can get llc to use 8 bytes alignment for APCS, I'm 
> 90% sure that this issue fires again.
> 
> Relative to APCS:
> 
> In APCS we can found the next statement:
> "The APCS does not define:
> - the layout in store of records, arrays, as so forth, used by 
> ARM-targeted compilers
> for C and C++
> ..."
> and a bit below:
> "This means that, the mapping from language-level data types and 
> arguments to APCS
> words is defined by each language implementation, not by the APCS."
> (APCS, 9.2.7 Data representation and argument passing)
> 
> APCS was defined in ARM Software Development Toolkit, ARM DUI 0041C.
> 
> In this document I also found next recommendation for compiler developers:
> "The alignment of a non-packed structure is the larger of:
> - The maximum alignment required by any of its fields
> ... "
> (3.2.4 Structured data types)