[PATCH] Fix AAPCS-VFP non-compliance when returning HFA from variadic function

[Oliver Stannard]

This patch is actually for clang, so I think I had the right mailing list.

There is already code in clang/lib/CodeGen/TargetInfo.cpp to detect
homogeneous aggregates, and it seems to be well tested by
clang/test/CodeGen/arm-homogenous.c.

The bug which I fixed was caused by the return being recognised as a
homogeneous aggregate, and marked as being passed in registers under
the assumption that VFP registers would be used, but since the
function was variadic the base standard was used instead. This caused
the return value to be passed in consecutive general purpose
registers, which is not valid in the base AAPCS.

Oliver

On 16 January 2014 11:32, Renato Golin <renato.golin at linaro.org> wrote:
> Hi Oliver,
>
> I'm moving your patch to the correct list (llvm-commits, not cfe-commits)
> and re-attaching the patch, for other reviews.
>
>
> On 16 January 2014 09:57, Oliver Stannard <oliver.stannard at arm.com> wrote:
>>
>> However, clang currently does not do the necessary transformation
>> to return the result in memory when using the AAPCS-VFP calling
>> convention,
>> and when the struct is a homogeneous aggregate of floating-point types.
>
>
> Nice catch! The patch looks good to me as it is, and I agree it's very
> important to fix this variadic bug, but I have a question regarding the ABI:
>
> 6.1.2.2 Result return
> Any result whose type would satisfy the conditions for a VFP CPRC is
> returned in the appropriate number of
> consecutive VFP registers starting with the lowest numbered register (s0,
> d0, q0).
>
> A VFP CPRC (as per 6.1.2.1), has some restriction on sizes like: "A
> Homogeneous Aggregate with a Base Type of a single- or double-precision
> floating-point type with one to
> four Elements.". Will the direct approach here account for that? Will it
> check the number of elements and revert to the base standard when more than
> 4?
>
> If not, I agree a fix would not be part of this patch, so in all cases, this
> specific patch looks good.
>
>
>>
>> Note, that this change will
>> break ABI backwards-compatibility with previous versions of clang/llvm.
>
>
> That's ok, we're in between releases, that's what we do! ;)
>
> cheers,
> --renato
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>