diff mbox

[4/7] target-arm: Refactor int-float conversions

Message ID 1304686095-30265-5-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell May 6, 2011, 12:48 p.m. UTC
The Neon versions of int-float conversions need their own helper routines
because they must use the "standard FPSCR" rather than the default one.
Refactor the helper functions to make it easy to add the neon versions.
While we're touching the code, move the helpers to op_helper.c so that
we can use the global env variable rather than passing it as a parameter.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c    |  125 ------------------------------------------------
 target-arm/helper.h    |   60 +++++++++++-----------
 target-arm/op_helper.c |   62 ++++++++++++++++++++++++
 target-arm/translate.c |   63 +++++++++++++-----------
 4 files changed, 127 insertions(+), 183 deletions(-)

Comments

Paul Brook May 6, 2011, 2:09 p.m. UTC | #1
> The Neon versions of int-float conversions need their own helper routines
> because they must use the "standard FPSCR" rather than the default one.
> Refactor the helper functions to make it easy to add the neon versions.
> While we're touching the code, move the helpers to op_helper.c so that
> we can use the global env variable rather than passing it as a parameter.

IMO this is going in the wrong direction.  We should in aiming for less 
implicit accesses to cpu state, not more.

Maybe better would be to explicitly pass a pointer the fp status. That way you 
don't even need separate VFP and NEON variants of these routines.

Paul
Peter Maydell May 6, 2011, 2:42 p.m. UTC | #2
On 6 May 2011 15:09, Paul Brook <paul@codesourcery.com> wrote:
>> The Neon versions of int-float conversions need their own helper routines
>> because they must use the "standard FPSCR" rather than the default one.
>> Refactor the helper functions to make it easy to add the neon versions.
>> While we're touching the code, move the helpers to op_helper.c so that
>> we can use the global env variable rather than passing it as a parameter.
>
> IMO this is going in the wrong direction.  We should in aiming for less
> implicit accesses to cpu state, not more.

I don't have a very strong feeling about this personally, I've just been
going in the direction suggested by past discussions eg
http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg00183.html

> Maybe better would be to explicitly pass a pointer the fp status. That way you
> don't even need separate VFP and NEON variants of these routines.

If you were otherwise going to pass in a CPUState pointer then just passing
the pointer to the fp_status is probably better, yes.

-- PMM
Blue Swirl May 6, 2011, 3:30 p.m. UTC | #3
On Fri, May 6, 2011 at 5:09 PM, Paul Brook <paul@codesourcery.com> wrote:
>> The Neon versions of int-float conversions need their own helper routines
>> because they must use the "standard FPSCR" rather than the default one.
>> Refactor the helper functions to make it easy to add the neon versions.
>> While we're touching the code, move the helpers to op_helper.c so that
>> we can use the global env variable rather than passing it as a parameter.
>
> IMO this is going in the wrong direction.  We should in aiming for less
> implicit accesses to cpu state, not more.

Performance wise global env variable is faster and the register is
always available. Do you mean that we should aim to get rid of special
status of global env, so that if no op uses it, it could be discarded
to free a register?

> Maybe better would be to explicitly pass a pointer the fp status. That way you
> don't even need separate VFP and NEON variants of these routines.

It would be nice to have generic float functions callable directly as
TCG helper.
Paul Brook May 6, 2011, 4:38 p.m. UTC | #4
> On Fri, May 6, 2011 at 5:09 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> The Neon versions of int-float conversions need their own helper
> >> routines because they must use the "standard FPSCR" rather than the
> >> default one. Refactor the helper functions to make it easy to add the
> >> neon versions. While we're touching the code, move the helpers to
> >> op_helper.c so that we can use the global env variable rather than
> >> passing it as a parameter.
> > 
> > IMO this is going in the wrong direction.  We should in aiming for less
> > implicit accesses to cpu state, not more.
> 
> Performance wise global env variable is faster and the register is
> always available. 

Not entirely true.  Reserving the global env variable has significant cost, 
especially on hosts with limited register sets (i.e. x86).  It's also a rather 
fragile hack.  There's a fairly long history of nasy hacks and things that 
just don't work in this context.  For example you can't reliably include 
stdio.h from these files, and they often break if you turn optimization off, 
which makes debugging much harder than it should be.

> Do you mean that we should aim to get rid of special
> status of global env, so that if no op uses it, it could be discarded
> to free a register?

That's already true for most of qemu.  IMO more interesting is being able to 
tell TCG that helpers don't mess with cpu state in opaque ways.  In theory 
it's already possible to do that manually. However that tends to be somewhat 
fragile, relying on careful maintenance and code code auditing, with mistakes 
triggering subtle hard-to-debug failures.  Much better to avoid the implicit 
global interface and make all helper arguments explicit.

> > Maybe better would be to explicitly pass a pointer the fp status. That
> > way you don't even need separate VFP and NEON variants of these
> > routines.
> 
> It would be nice to have generic float functions callable directly as
> TCG helper.

Possibly.  I'd have to look quite a bit closer to determine whether exposing 
the generic float functions is useful in practice, or if you still end up 
needing wrappers in most cases for most targets.  Adding "native" floating 
point support to the TCG interface is also a possibility.  In practice this 
might end up as wrappers around helper functions, but might provide a nicer 
programming interface.

Paul
Blue Swirl May 8, 2011, 10:32 a.m. UTC | #5
On Fri, May 6, 2011 at 7:38 PM, Paul Brook <paul@codesourcery.com> wrote:
>> On Fri, May 6, 2011 at 5:09 PM, Paul Brook <paul@codesourcery.com> wrote:
>> >> The Neon versions of int-float conversions need their own helper
>> >> routines because they must use the "standard FPSCR" rather than the
>> >> default one. Refactor the helper functions to make it easy to add the
>> >> neon versions. While we're touching the code, move the helpers to
>> >> op_helper.c so that we can use the global env variable rather than
>> >> passing it as a parameter.
>> >
>> > IMO this is going in the wrong direction.  We should in aiming for less
>> > implicit accesses to cpu state, not more.
>>
>> Performance wise global env variable is faster and the register is
>> always available.
>
> Not entirely true.  Reserving the global env variable has significant cost,
> especially on hosts with limited register sets (i.e. x86).  It's also a rather
> fragile hack.  There's a fairly long history of nasy hacks and things that
> just don't work in this context.  For example you can't reliably include
> stdio.h from these files, and they often break if you turn optimization off,
> which makes debugging much harder than it should be.

Even if we don't reserve the register, in many cases a corresponding
pointer to CPUState will be needed. But there will still be the
advantage that this temporary pointer can be discarded while the
globally reserved register is reserved forever.

>> Do you mean that we should aim to get rid of special
>> status of global env, so that if no op uses it, it could be discarded
>> to free a register?
>
> That's already true for most of qemu.  IMO more interesting is being able to
> tell TCG that helpers don't mess with cpu state in opaque ways.  In theory
> it's already possible to do that manually. However that tends to be somewhat
> fragile, relying on careful maintenance and code code auditing, with mistakes
> triggering subtle hard-to-debug failures.  Much better to avoid the implicit
> global interface and make all helper arguments explicit.

OK. This will be a major refactoring, but given the expected
performance increase, it should be done.

>> > Maybe better would be to explicitly pass a pointer the fp status. That
>> > way you don't even need separate VFP and NEON variants of these
>> > routines.
>>
>> It would be nice to have generic float functions callable directly as
>> TCG helper.
>
> Possibly.  I'd have to look quite a bit closer to determine whether exposing
> the generic float functions is useful in practice, or if you still end up
> needing wrappers in most cases for most targets.  Adding "native" floating
> point support to the TCG interface is also a possibility.  In practice this
> might end up as wrappers around helper functions, but might provide a nicer
> programming interface.
>
> Paul
>
Aurelien Jarno May 14, 2011, 10:38 p.m. UTC | #6
On Sun, May 08, 2011 at 01:32:34PM +0300, Blue Swirl wrote:
> On Fri, May 6, 2011 at 7:38 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> On Fri, May 6, 2011 at 5:09 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> >> The Neon versions of int-float conversions need their own helper
> >> >> routines because they must use the "standard FPSCR" rather than the
> >> >> default one. Refactor the helper functions to make it easy to add the
> >> >> neon versions. While we're touching the code, move the helpers to
> >> >> op_helper.c so that we can use the global env variable rather than
> >> >> passing it as a parameter.
> >> >
> >> > IMO this is going in the wrong direction.  We should in aiming for less
> >> > implicit accesses to cpu state, not more.
> >>
> >> Performance wise global env variable is faster and the register is
> >> always available.
> >
> > Not entirely true.  Reserving the global env variable has significant cost,
> > especially on hosts with limited register sets (i.e. x86).  It's also a rather
> > fragile hack.  There's a fairly long history of nasy hacks and things that
> > just don't work in this context.  For example you can't reliably include
> > stdio.h from these files, and they often break if you turn optimization off,
> > which makes debugging much harder than it should be.
> 
> Even if we don't reserve the register, in many cases a corresponding
> pointer to CPUState will be needed. But there will still be the
> advantage that this temporary pointer can be discarded while the
> globally reserved register is reserved forever.
> 
> >> Do you mean that we should aim to get rid of special
> >> status of global env, so that if no op uses it, it could be discarded
> >> to free a register?
> >
> > That's already true for most of qemu.  IMO more interesting is being able to
> > tell TCG that helpers don't mess with cpu state in opaque ways.  In theory
> > it's already possible to do that manually. However that tends to be somewhat
> > fragile, relying on careful maintenance and code code auditing, with mistakes
> > triggering subtle hard-to-debug failures.  Much better to avoid the implicit
> > global interface and make all helper arguments explicit.
> 
> OK. This will be a major refactoring, but given the expected
> performance increase, it should be done.
> 

We might want to do it from the cleanliness point of view, but i really
doubt we should expect performance increase from this (actually i think
it will be the contrary).
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index f072527..de00468 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2526,100 +2526,6 @@  DO_VFP_cmp(s, float32)
 DO_VFP_cmp(d, float64)
 #undef DO_VFP_cmp
 
-/* Integer to float conversion.  */
-float32 VFP_HELPER(uito, s)(uint32_t x, CPUState *env)
-{
-    return uint32_to_float32(x, &env->vfp.fp_status);
-}
-
-float64 VFP_HELPER(uito, d)(uint32_t x, CPUState *env)
-{
-    return uint32_to_float64(x, &env->vfp.fp_status);
-}
-
-float32 VFP_HELPER(sito, s)(uint32_t x, CPUState *env)
-{
-    return int32_to_float32(x, &env->vfp.fp_status);
-}
-
-float64 VFP_HELPER(sito, d)(uint32_t x, CPUState *env)
-{
-    return int32_to_float64(x, &env->vfp.fp_status);
-}
-
-/* Float to integer conversion.  */
-uint32_t VFP_HELPER(toui, s)(float32 x, CPUState *env)
-{
-    if (float32_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float32_to_uint32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(toui, d)(float64 x, CPUState *env)
-{
-    if (float64_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float64_to_uint32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosi, s)(float32 x, CPUState *env)
-{
-    if (float32_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float32_to_int32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosi, d)(float64 x, CPUState *env)
-{
-    if (float64_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float64_to_int32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(touiz, s)(float32 x, CPUState *env)
-{
-    if (float32_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float32_to_uint32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(touiz, d)(float64 x, CPUState *env)
-{
-    if (float64_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float64_to_uint32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosiz, s)(float32 x, CPUState *env)
-{
-    if (float32_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float32_to_int32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
-{
-    if (float64_is_any_nan(x)) {
-        float_raise(float_flag_invalid, &env->vfp.fp_status);
-        return 0;
-    }
-    return float64_to_int32_round_to_zero(x, &env->vfp.fp_status);
-}
-
 /* floating point conversion */
 float64 VFP_HELPER(fcvtd, s)(float32 x, CPUState *env)
 {
@@ -2639,37 +2545,6 @@  float32 VFP_HELPER(fcvts, d)(float64 x, CPUState *env)
     return float32_maybe_silence_nan(r);
 }
 
-/* VFP3 fixed point conversion.  */
-#define VFP_CONV_FIX(name, p, fsz, itype, sign) \
-float##fsz VFP_HELPER(name##to, p)(uint##fsz##_t  x, uint32_t shift, \
-                                   CPUState *env) \
-{ \
-    float##fsz tmp; \
-    tmp = sign##int32_to_##float##fsz ((itype##_t)x, &env->vfp.fp_status); \
-    return float##fsz##_scalbn(tmp, -(int)shift, &env->vfp.fp_status); \
-} \
-uint##fsz##_t VFP_HELPER(to##name, p)(float##fsz x, uint32_t shift, \
-                                      CPUState *env) \
-{ \
-    float##fsz tmp; \
-    if (float##fsz##_is_any_nan(x)) { \
-        float_raise(float_flag_invalid, &env->vfp.fp_status); \
-        return 0; \
-    } \
-    tmp = float##fsz##_scalbn(x, shift, &env->vfp.fp_status); \
-    return float##fsz##_to_##itype##_round_to_zero(tmp, &env->vfp.fp_status); \
-}
-
-VFP_CONV_FIX(sh, d, 64, int16, )
-VFP_CONV_FIX(sl, d, 64, int32, )
-VFP_CONV_FIX(uh, d, 64, uint16, u)
-VFP_CONV_FIX(ul, d, 64, uint32, u)
-VFP_CONV_FIX(sh, s, 32, int16, )
-VFP_CONV_FIX(sl, s, 32, int32, )
-VFP_CONV_FIX(uh, s, 32, uint16, u)
-VFP_CONV_FIX(ul, s, 32, uint32, u)
-#undef VFP_CONV_FIX
-
 /* Half precision conversions.  */
 static float32 do_fcvt_f16_to_f32(uint32_t a, CPUState *env, float_status *s)
 {
diff --git a/target-arm/helper.h b/target-arm/helper.h
index ae701e8..2c54d5e 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -96,36 +96,36 @@  DEF_HELPER_3(vfp_cmped, void, f64, f64, env)
 DEF_HELPER_2(vfp_fcvtds, f64, f32, env)
 DEF_HELPER_2(vfp_fcvtsd, f32, f64, env)
 
-DEF_HELPER_2(vfp_uitos, f32, i32, env)
-DEF_HELPER_2(vfp_uitod, f64, i32, env)
-DEF_HELPER_2(vfp_sitos, f32, i32, env)
-DEF_HELPER_2(vfp_sitod, f64, i32, env)
-
-DEF_HELPER_2(vfp_touis, i32, f32, env)
-DEF_HELPER_2(vfp_touid, i32, f64, env)
-DEF_HELPER_2(vfp_touizs, i32, f32, env)
-DEF_HELPER_2(vfp_touizd, i32, f64, env)
-DEF_HELPER_2(vfp_tosis, i32, f32, env)
-DEF_HELPER_2(vfp_tosid, i32, f64, env)
-DEF_HELPER_2(vfp_tosizs, i32, f32, env)
-DEF_HELPER_2(vfp_tosizd, i32, f64, env)
-
-DEF_HELPER_3(vfp_toshs, i32, f32, i32, env)
-DEF_HELPER_3(vfp_tosls, i32, f32, i32, env)
-DEF_HELPER_3(vfp_touhs, i32, f32, i32, env)
-DEF_HELPER_3(vfp_touls, i32, f32, i32, env)
-DEF_HELPER_3(vfp_toshd, i64, f64, i32, env)
-DEF_HELPER_3(vfp_tosld, i64, f64, i32, env)
-DEF_HELPER_3(vfp_touhd, i64, f64, i32, env)
-DEF_HELPER_3(vfp_tould, i64, f64, i32, env)
-DEF_HELPER_3(vfp_shtos, f32, i32, i32, env)
-DEF_HELPER_3(vfp_sltos, f32, i32, i32, env)
-DEF_HELPER_3(vfp_uhtos, f32, i32, i32, env)
-DEF_HELPER_3(vfp_ultos, f32, i32, i32, env)
-DEF_HELPER_3(vfp_shtod, f64, i64, i32, env)
-DEF_HELPER_3(vfp_sltod, f64, i64, i32, env)
-DEF_HELPER_3(vfp_uhtod, f64, i64, i32, env)
-DEF_HELPER_3(vfp_ultod, f64, i64, i32, env)
+DEF_HELPER_1(vfp_uitos, f32, i32)
+DEF_HELPER_1(vfp_uitod, f64, i32)
+DEF_HELPER_1(vfp_sitos, f32, i32)
+DEF_HELPER_1(vfp_sitod, f64, i32)
+
+DEF_HELPER_1(vfp_touis, i32, f32)
+DEF_HELPER_1(vfp_touid, i32, f64)
+DEF_HELPER_1(vfp_touizs, i32, f32)
+DEF_HELPER_1(vfp_touizd, i32, f64)
+DEF_HELPER_1(vfp_tosis, i32, f32)
+DEF_HELPER_1(vfp_tosid, i32, f64)
+DEF_HELPER_1(vfp_tosizs, i32, f32)
+DEF_HELPER_1(vfp_tosizd, i32, f64)
+
+DEF_HELPER_2(vfp_toshs, i32, f32, i32)
+DEF_HELPER_2(vfp_tosls, i32, f32, i32)
+DEF_HELPER_2(vfp_touhs, i32, f32, i32)
+DEF_HELPER_2(vfp_touls, i32, f32, i32)
+DEF_HELPER_2(vfp_toshd, i64, f64, i32)
+DEF_HELPER_2(vfp_tosld, i64, f64, i32)
+DEF_HELPER_2(vfp_touhd, i64, f64, i32)
+DEF_HELPER_2(vfp_tould, i64, f64, i32)
+DEF_HELPER_2(vfp_shtos, f32, i32, i32)
+DEF_HELPER_2(vfp_sltos, f32, i32, i32)
+DEF_HELPER_2(vfp_uhtos, f32, i32, i32)
+DEF_HELPER_2(vfp_ultos, f32, i32, i32)
+DEF_HELPER_2(vfp_shtod, f64, i64, i32)
+DEF_HELPER_2(vfp_sltod, f64, i64, i32)
+DEF_HELPER_2(vfp_uhtod, f64, i64, i32)
+DEF_HELPER_2(vfp_ultod, f64, i64, i32)
 
 DEF_HELPER_2(vfp_fcvt_f16_to_f32, f32, i32, env)
 DEF_HELPER_2(vfp_fcvt_f32_to_f16, i32, f32, env)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 8334fbc..1afea43 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -424,3 +424,65 @@  uint32_t HELPER(ror_cc)(uint32_t x, uint32_t i)
         return ((uint32_t)x >> shift) | (x << (32 - shift));
     }
 }
+
+/* Integer to float and float to integer conversions */
+
+#define CONV_ITOF(name, fsz, sign, fpst) \
+float##fsz HELPER(name)(uint32_t x) \
+{ \
+    return sign##int32_to_##float##fsz(x, fpst); \
+}
+
+#define CONV_FTOI(name, fsz, sign, fpst, round) \
+uint32_t HELPER(name)(float##fsz x) \
+{ \
+    if (float##fsz##_is_any_nan(x)) { \
+        float_raise(float_flag_invalid, fpst); \
+        return 0; \
+    } \
+    return float##fsz##_to_##sign##int32##round(x, fpst); \
+}
+
+#define VFP_CONVS(name, p, fsz, sign) \
+CONV_ITOF(vfp_##name##to##p, fsz, sign, &env->vfp.fp_status) \
+CONV_FTOI(vfp_##to##name##p, fsz, sign, &env->vfp.fp_status, ) \
+CONV_FTOI(vfp_##to##name##z##p, fsz, sign, &env->vfp.fp_status, _round_to_zero)
+
+VFP_CONVS(si, s, 32, )
+VFP_CONVS(si, d, 64, )
+VFP_CONVS(ui, s, 32, u)
+VFP_CONVS(ui, d, 64, u)
+
+#undef CONV_ITOF
+#undef CONV_FTOI
+#undef VFP_CONVS
+
+/* VFP3 fixed point conversion.  */
+#define VFP_CONV_FIX(pfx, name, p, fsz, itype, sign, status) \
+float##fsz HELPER(pfx##name##to##p)(uint##fsz##_t  x, uint32_t shift) \
+{ \
+    float##fsz tmp; \
+    tmp = sign##int32_to_##float##fsz((itype##_t)x, status); \
+    return float##fsz##_scalbn(tmp, -(int)shift, status); \
+} \
+uint##fsz##_t HELPER(pfx##to##name##p)(float##fsz x, uint32_t shift) \
+{ \
+    float##fsz tmp; \
+    if (float##fsz##_is_any_nan(x)) { \
+        float_raise(float_flag_invalid, status); \
+        return 0; \
+    } \
+    tmp = float##fsz##_scalbn(x, shift, status); \
+    return float##fsz##_to_##itype##_round_to_zero(tmp, status); \
+}
+
+VFP_CONV_FIX(vfp_, sh, d, 64, int16, , &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, sl, d, 64, int32, , &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, uh, d, 64, uint16, u, &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, ul, d, 64, uint32, u, &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, sh, s, 32, int16, , &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, sl, s, 32, int32, , &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, uh, s, 32, uint16, u, &env->vfp.fp_status)
+VFP_CONV_FIX(vfp_, ul, s, 32, uint32, u, &env->vfp.fp_status)
+
+#undef VFP_CONV_FIX
diff --git a/target-arm/translate.c b/target-arm/translate.c
index a1af436..195cf30 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -959,60 +959,67 @@  static inline void gen_vfp_F1_ld0(int dp)
 
 static inline void gen_vfp_uito(int dp)
 {
-    if (dp)
-        gen_helper_vfp_uitod(cpu_F0d, cpu_F0s, cpu_env);
-    else
-        gen_helper_vfp_uitos(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_uitod(cpu_F0d, cpu_F0s);
+    } else {
+        gen_helper_vfp_uitos(cpu_F0s, cpu_F0s);
+    }
 }
 
 static inline void gen_vfp_sito(int dp)
 {
-    if (dp)
-        gen_helper_vfp_sitod(cpu_F0d, cpu_F0s, cpu_env);
-    else
-        gen_helper_vfp_sitos(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_sitod(cpu_F0d, cpu_F0s);
+    } else {
+        gen_helper_vfp_sitos(cpu_F0s, cpu_F0s);
+    }
 }
 
 static inline void gen_vfp_toui(int dp)
 {
-    if (dp)
-        gen_helper_vfp_touid(cpu_F0s, cpu_F0d, cpu_env);
-    else
-        gen_helper_vfp_touis(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_touid(cpu_F0s, cpu_F0d);
+    } else {
+        gen_helper_vfp_touis(cpu_F0s, cpu_F0s);
+    }
 }
 
 static inline void gen_vfp_touiz(int dp)
 {
-    if (dp)
-        gen_helper_vfp_touizd(cpu_F0s, cpu_F0d, cpu_env);
-    else
-        gen_helper_vfp_touizs(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_touizd(cpu_F0s, cpu_F0d);
+    } else {
+        gen_helper_vfp_touizs(cpu_F0s, cpu_F0s);
+    }
 }
 
 static inline void gen_vfp_tosi(int dp)
 {
-    if (dp)
-        gen_helper_vfp_tosid(cpu_F0s, cpu_F0d, cpu_env);
-    else
-        gen_helper_vfp_tosis(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_tosid(cpu_F0s, cpu_F0d);
+    } else {
+        gen_helper_vfp_tosis(cpu_F0s, cpu_F0s);
+    }
 }
 
 static inline void gen_vfp_tosiz(int dp)
 {
-    if (dp)
-        gen_helper_vfp_tosizd(cpu_F0s, cpu_F0d, cpu_env);
-    else
-        gen_helper_vfp_tosizs(cpu_F0s, cpu_F0s, cpu_env);
+    if (dp) {
+        gen_helper_vfp_tosizd(cpu_F0s, cpu_F0d);
+    } else {
+        gen_helper_vfp_tosizs(cpu_F0s, cpu_F0s);
+    }
 }
 
 #define VFP_GEN_FIX(name) \
 static inline void gen_vfp_##name(int dp, int shift) \
 { \
     TCGv tmp_shift = tcg_const_i32(shift); \
-    if (dp) \
-        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
-    else \
-        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
+    if (dp) { \
+        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift); \
+    } else { \
+        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift); \
+    } \
     tcg_temp_free_i32(tmp_shift); \
 }
 VFP_GEN_FIX(tosh)