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

[James Molloy]

Hi Stepan,

I would normally have just suggested setting the test to use a non-eabi
triple, but it sounds like you've modified the test to be better and
have more test coverage anyway, so that's better.

My bad - looks like the ABI in ARMSubtarget can only be one of
ARM_ABI_APCS or ARM_ABI_AAPCS, so aapcs-vfp would be covered by
ARM_ABI_AAPCS - Anton, is that right?

Assuming Anton agrees, the patch looks good to me.

Cheers,

James

On Fri, 2012-10-12 at 15:47 +0100, Stepan Dyatkovskiy wrote:
> Hi James,
> 
> >    * Why the need to change the LDRB_POST_IMM-Crash.ll testcase so
> > drastically? (changing from doubles to i8s)
> 
> For non EABI mode COPY_STRUCT_BYVAL is expanded onto set of ldr/str (or 
> vld1/vst1 for EABI) instructions,
> But if there are some remainder, less than copy-unit-size (for vld1 it 
> is 8 bytes) it copied with ldrb/strb instructions (see 
> ARMTargetLowering::EmitStructByval for more details).
> 
> Current test uses (i32, { double, double, double } ) params set. Params 
> are passed with next way:
> 1. i32 -> r0
> 2. First three 32 bit words of struct -> r1,r2,r3 (12 bytes)
>     Next 56 bytes copied with 7 vld1/vst1 instructions.
>     Remainder (4 bytes) copied with ldrb/strb instructions.
> 
> But for AAPCS mode we shouldn't use r1,r2,r3 set for head part of 
> structure. We can use r2,r3 only. So bug-fix changes output code for 
> this test. After this patch params will be passed with next way:
> 1. i32 ->r0
> 2. First *two* 32 bit words of struct -> r2,r3 (8 bytes)
>     Next 64 bytes are copied with 8 vld1/vst1 instructions.
>     No bytes remained. No remainder => no ldrb instructions.
> 
> IMHO the best way to get llvm to emit ldrb instructions is to form 1 
> byte aligned structure and then to get it to emit strictly 1 byte into 
> memory.
> 
> I changed param set for function to (i32, i32, i32, { i8 <-- 5 times }).
> It get llvm to pass params with next way:
> 1. i32 -> r0
> 2. i32 -> r1
> 3. i32 -> r2
> 4. i8 i8 i8 i8 -> r3
>     We have one byte remained -> emit ldrb/strb
> 
> Note, it works for any mode (EABI/non-EABI), since biggest field in 
> structure is 8 bit length, that makes it 1 byte aligned.
> 
> And... Current test contains some extra nodes: memcpy, alloca, etc 
> (since it was made with bugpoint and I cleaned it then inaccurately). 
> And when I was writing that test I wasn't pretty familiar with this part 
> of codegen. Now I decided to make it from scratch: as simplest as it 
> possible.
> 
> >    * if (Subtarget->isAAPCS_ABI()  <-- surely this should be enabled when
> > using the aapcs-vfp ABI too?
> aapcs-vfp = AAPCS mode + enable VFP extension, right? The answer is 
> "yes" then. How to check that aapcs-vfp is used?
> 
> -Stepan.
> 
>