Re: [Qemu-devel] [PATCH 01/10] target-arm: Make Neon helper routines use correct FP status

[Blue Swirl]

On Sun, Apr 3, 2011 at 1:51 PM, Peter Maydell <address@hidden> wrote:
> On 3 April 2011 10:41, Blue Swirl <address@hidden> wrote:
>> On Sat, Apr 2, 2011 at 1:33 AM, Peter Maydell <address@hidden> wrote:
>>> Nobody seemed to have a coherent rule about when functions
>>> should be in op_helper.c and use the global 'env' variable
>>> and when they should be in some other file and have 'env'
>>> passed as a parameter
>
>> In general, helpers for the translated code belong to op_helper.c.
>> They can and should access global env directly for speed. If a helper
>> is used for both translated code and outside of it, a wrapper should
>> be added to do global env shuffling (or for example a copy without
>> shuffling added).
>
> OK, we can do that, but at the moment "helper function not in
> op_helper.c" is hugely in the majority so there's a lot of
> code we'd be moving around:
>
> $ grep -c HELPER target-arm/*.c
> target-arm/helper.c:68
> target-arm/iwmmxt_helper.c:59
> target-arm/machine.c:0
> target-arm/neon_helper.c:103
> target-arm/op_helper.c:28
> target-arm/translate.c:2
>
> (ie just 10% or so of ARM helper functions are in op_helper.c)
>
> ...and this cleanup would basically amount to folding
> neon_helper.c, iwmmxt_helper.c and bits of helper.c into
> op_helper.c (and then removing the 'env' parameters, so
> a big patch to translate.c as well, which I don't suppose
> anybody maintaining an out-of-tree target-arm patchset will
> thank us for :-)).

Alternatively those files could be compiled with HELPER_CFLAGS. In
either case, the code would have to be checked for 'env' usage and
adjusted.

> But I can submit a patch to do that if it's the right thing.

It's not so much about correctness, but performance. All generated
code already has access to global env, so passing it via helper
arguments requires extra instructions which can be avoided.