[llvm-commits] PR6059 patch

[Bob Wilson]

On Jan 21, 2010, at 9:42 AM, Rafael Espindola wrote:

> Attached is a potential fix for PR6059. One way to look at it is that
> it is a partial revert of Bob's previous patch to fix PR5406. Before
> that patch llvm-gcc would use i64 and that would force the arguments
> to be correctly aligned. What this patch does is check if it is OK to
> use i64. If it is not, we produce an i32 padding.

It seems wrong to be putting these kinds of target-specific details in target-independent code.  It is fine to have the generic code insert padding, but that ought to be done based on some target hook that identifies when padding is needed.

Your check for "ScalarElts.size() % 2" looks wrong.  There may be other arguments before the aggregate and some of them may be 64-bits and occupy 2 registers; or, with the hard-float variant of AAPCS, some of the preceding arguments may be passed in VFP registers.  Even if it was correct, it is too ARM-specific to put in the generic code.

I really think this needs to be handled in llvm_arm_should_pass_aggregate_in_mixed_regs().

> I have done a quick testing with this patch and
> 
> SingleSource/UnitTests/2003-05-07-VarArgs
> SingleSource/UnitTests/2003-08-11-VaListArg
> 
> are fixed on AAPCS.
> 
> Bob, I will do a full test on AAPCS (with qemu). I can test on linux
> x86-64 too, but I don't think this code path is used in there. Can you
> help me test this? Is there another test I can make with qemu?

I ran the GCC compat tests for arm-apple-darwin (APCS) with your patch.  There are many regressions:

FAIL: gcc.dg/compat/scalar-by-value-3 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-3 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-by-value-3 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-4 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-by-value-4 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-5 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-5 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-by-value-5 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-6 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-6 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-by-value-6 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-return-3 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-return-3 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-return-3 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-return-4 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/scalar-return-4 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-10 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-10 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-10 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-13 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-13 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-13 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-14 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-14 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-14 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-15 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-16 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-16 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-16 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-17 c_compat_x_alt.o-c_compat_y_tst.o execute
FAIL: gcc.dg/compat/struct-by-value-18 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-19 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-19 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-19 c_compat_x_alt.o-c_compat_y_tst.o execute
FAIL: gcc.dg/compat/struct-by-value-20 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-20 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-20 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-21 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-21 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-21 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-2 c_compat_x_tst.o compile,  (internal compiler error)
FAIL: gcc.dg/compat/struct-by-value-2 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-3 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-3 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-3 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-4 c_compat_x_tst.o compile,  (internal compiler error)
FAIL: gcc.dg/compat/struct-by-value-4 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-5a c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-5a c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-5a c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-5b c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-5b c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-5b c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-6a c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-6a c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-6b c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-6b c_compat_x_tst.o-c_compat_y_alt.o execute
FAIL: gcc.dg/compat/struct-by-value-7a c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-7a c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-by-value-7b c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-7b c_compat_x_tst.o-c_compat_y_alt.o execute
FAIL: gcc.dg/compat/struct-by-value-8 c_compat_x_tst.o compile,  (internal compiler error)
FAIL: gcc.dg/compat/struct-by-value-9 c_compat_x_tst.o compile,  (internal compiler error)
FAIL: gcc.dg/compat/struct-return-10 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-10 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-return-19 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-19 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-return-20 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-20 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-return-21 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-21 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/struct-return-2 c_compat_x_tst.o compile,  (internal compiler error)
FAIL: gcc.dg/compat/struct-return-3 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-3 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/union-by-value-1 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/union-by-value-1 c_compat_x_tst.o-c_compat_y_alt.o execute 
FAIL: gcc.dg/compat/union-by-value-1 c_compat_x_alt.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/union-return-1 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/union-return-1 c_compat_x_tst.o-c_compat_y_alt.o execute