Re: [PATCH v2 2/9] target/ppc: Implemented vector divide instructions

[Lucas Mateus Martins Araujo e Castro]

On 11/04/2022 22:51, Richard Henderson wrote:

>       On 4/5/22 12:55, Lucas Mateus Castro(alqotel) wrote:
>       
>       
> > +
> >         
> >         +#define DO_VDIV_VMOD(NAME, SZ, DIV,
> >         SIGNED)                             \
> >         
> >         +static void NAME(TCGv_i##SZ t, TCGv_i##SZ a, TCGv_i##SZ
> >         b)              \
> >         
> > +{                                                                      
> >         \
> >         
> >         +   
> >         /*                                                                 
> >         \
> >         
> >         +     *  If N/0 the instruction used by the backend might
> >         deliver        \
> >         
> >         +     *  an invalid division signal to the process, so if b = 0
> >         return   \
> >         
> >         +     *  N/1 and if signed instruction, the same for a =
> >         int_min, b = -1 \
> >         
> >         +    
> >         */                                                                
> >         \
> >         
> >         +    if (SIGNED)
> >         {                                                       \
> >         
> >         +        TCGv_i##SZ t0 =
> >         tcg_temp_new_i##SZ();                           \
> >         
> >         +        TCGv_i##SZ t1 =
> >         tcg_temp_new_i##SZ();                           \
> >         
> >         +        tcg_gen_setcondi_i##SZ(TCG_COND_EQ, t0, a,
> >         INT##SZ##_MIN);      \
> >         
> >         +        tcg_gen_setcondi_i##SZ(TCG_COND_EQ, t1, b,
> >         -1);                 \
> >         
> >         +        tcg_gen_and_i##SZ(t0, t0,
> >         t1);                                  \
> >         
> >         +        tcg_gen_setcondi_i##SZ(TCG_COND_EQ, t1, b,
> >         0);                  \
> >         
> >         +        tcg_gen_or_i##SZ(t0, t0,
> >         t1);                                   \
> >         
> >         +        tcg_gen_movi_i##SZ(t1,
> >         0);                                      \
> >         
> >         +        tcg_gen_movcond_i##SZ(TCG_COND_NE, b, t0, t1, t0,
> >         b);           \
> >         
> >         +        DIV(t, a,
> >         b);                                                   \
> >         
> >         +       
> >         tcg_temp_free_i##SZ(t0);                                       
> >         \
> >         
> >         +       
> >         tcg_temp_free_i##SZ(t1);                                       
> >         \
> >         
> >         +    } else
> >         {                                                            \
> >         
> >         +        TCGv_i##SZ zero =
> >         tcg_constant_i##SZ(0);                        \
> >         
> >         +        TCGv_i##SZ _one_ =
> >         tcg_constant_i##SZ(1);                         \
> >         
> >         +        tcg_gen_movcond_i##SZ(TCG_COND_EQ, b, b, zero, one,
> >         b);         \
> >         
> >         +        DIV(t, a,
> >         b);                                                   \
> >         
> >         +   
> >         }                                                                  
> >         \
> >         
> >         +}
> >         
> >       
>       
>       This is overkill.  Even if you keep some macros, passing in SIGNED
>       and using it in the
>       
>       outermost if is a sign you should split the macro in two.
>       
>       
>       However, only tcg_gen_div_i64 really requires the full signed
>       treatment; tcg_gen_div_i32
>       
>       can be better handled by extending to i64, because INT32_MIN /
>       -1ULL does not trap.
>       
>       
>       I think this would be much easier to read as 4 separate functions.
>       
>       
>       
>     
    Ok, I'll change it to 4 different macros, move clz128 to int128.h
    and turn TRANS_VDIV_VMOD into do_vdiv_vmod function and call it with
    TRANS() in v3 
> r~
>       
>

--
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer