Re: [Qemu-devel] [PATCH 04/10] tcg: Use helper-gen.h in tcg-op.h

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> No need to open-code the setup of the builtin helpers.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  tcg/tcg-op.h | 175 
> +++++++++--------------------------------------------------
>  1 file changed, 25 insertions(+), 150 deletions(-)
>
> diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
> index 8560695..f4367a0 100644
> --- a/tcg/tcg-op.h
> +++ b/tcg/tcg-op.h
> @@ -24,6 +24,14 @@
>  #include "tcg.h"
>  #include "exec/helper-proto.h"
>  
> +static inline void tcg_gen_helperN(void *func, int flags, int sizemask,
> +                                   TCGArg ret, int nargs, TCGArg *args)
> +{
> +    tcg_gen_callN(&tcg_ctx, func, flags, sizemask, ret, nargs, args);
> +}
> +
> +#include "exec/helper-gen.h"
> +
<snip>
>  
> -/* A version of dh_sizemask from def-helper.h that doesn't rely on
> -   preprocessor magic.  */
> -static inline int tcg_gen_sizemask(int n, int is_64bit, int is_signed)
> -{
> -    return (is_64bit << n*2) | (is_signed << (n*2 + 1));
> -}
> -
> -/* helper calls */
> -static inline void tcg_gen_helperN(void *func, int flags, int sizemask,
> -                                   TCGArg ret, int nargs, TCGArg *args)
> -{
> -    tcg_gen_callN(&tcg_ctx, func, flags, sizemask, ret, nargs, args);
> -}
> -
<snip>
>  
>  static inline void tcg_gen_ld8u_i32(TCGv_i32 ret, TCGv_ptr arg2, 
> tcg_target_long offset)
> @@ -708,12 +675,7 @@ static inline void tcg_gen_div_i32(TCGv_i32 ret, 
> TCGv_i32 arg1, TCGv_i32 arg2)
>          tcg_gen_op5_i32(INDEX_op_div2_i32, ret, t0, arg1, t0, arg2);
>          tcg_temp_free_i32(t0);
>      } else {
> -        int sizemask = 0;
> -        /* Return value and both arguments are 32-bit and signed.  */
> -        sizemask |= tcg_gen_sizemask(0, 0, 1);
> -        sizemask |= tcg_gen_sizemask(1, 0, 1);
> -        sizemask |= tcg_gen_sizemask(2, 0, 1);
> -        tcg_gen_helper32(helper_div_i32, sizemask, ret, arg1, arg2);
> +        gen_helper_div_i32(ret, arg1, arg2);
>      }
>  }

I'm having trouble figuring out what the sizemask stuff was trying to
achieve and if it's required/covered after the change. Is the final
helper now the one defined as?

DEF(div_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_div_i32))

it's hard to follow the parameters of this compared to the target
helpers where the the argument sizes are a lot more explicit.

-- 
Alex Bennée