Re: [Qemu-devel] [PATCH v2 6/6] tcg/mips: Support r6 SEL{NE, EQ}Z instead of MOVN/MOVZ

[James Hogan]

On Fri, Oct 02, 2015 at 05:53:37AM +1000, Richard Henderson wrote:
> On 10/01/2015 08:58 PM, James Hogan wrote:
> > Extend MIPS movcond implementation to support the SELNEZ/SELEQZ
> > instructions introduced in MIPS r6 (where MOVN/MOVZ have been removed).
> >
> > Whereas the "MOVN/MOVZ rd, rs, rt" instructions have the following
> > semantics:
> >   rd = [!]rt ? rs : rd
> >
> > The "SELNEZ/SELEQZ rd, rs, rt" instructions are slightly different:
> >   rd = [!]rt ? rs : 0
> >
> > First we ensure that if one of the movcond input values is zero that it
> > comes last (we can swap the input arguments if we invert the condition).
> > This is so that it can exactly match one of the SELNEZ/SELEQZ
> > instructions and avoid the need to emit the other one.
> >
> > Otherwise we emit the opposite instruction first into a temporary
> > register, and OR that into the result:
> >   SELNEZ/SELEQZ  TMP1, v2, c1
> >   SELEQZ/SELNEZ  ret, v1, c1
> >   OR             ret, ret, TMP1
> >
> > Which does the following:
> >   ret = cond ? v1 : v2
> >
> > Signed-off-by: James Hogan <address@hidden>
> > Cc: Richard Henderson <address@hidden>
> > Cc: Aurelien Jarno <address@hidden>
> > ---
> > Changes in v2:
> > - Combine with patch 6 from v1, and drop functional changes to movcond
> >    implementation pre-r6. We now provide different constraints for
> >    movcond depending on presence of r6. (thanks Richard for feedback).
> > ---
> >   tcg/mips/tcg-target.c | 40 ++++++++++++++++++++++++++++++++++------
> >   1 file changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> > index a937b1475d04..ef1e85a75e22 100644
> > --- a/tcg/mips/tcg-target.c
> > +++ b/tcg/mips/tcg-target.c
> > @@ -314,6 +314,8 @@ typedef enum {
> >       OPC_NOR      = OPC_SPECIAL | 0x27,
> >       OPC_SLT      = OPC_SPECIAL | 0x2A,
> >       OPC_SLTU     = OPC_SPECIAL | 0x2B,
> > +    OPC_SELEQZ   = OPC_SPECIAL | 0x35,
> > +    OPC_SELNEZ   = OPC_SPECIAL | 0x37,
> >
> >       OPC_REGIMM   = 0x01 << 26,
> >       OPC_BLTZ     = OPC_REGIMM | (0x00 << 16),
> > @@ -858,13 +860,24 @@ static void tcg_out_brcond2(TCGContext *s, TCGCond 
> > cond, TCGReg al, TCGReg ah,
> >   }
> >
> >   static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
> > -                            TCGReg c1, TCGReg c2, TCGReg v)
> > +                            TCGReg c1, TCGReg c2, TCGReg v1, TCGReg v2)
> >   {
> > -    MIPSInsn m_opc = OPC_MOVN;
> > +    MIPSInsn m_opc_t = use_mips32r6_instructions ? OPC_SELNEZ : OPC_MOVN;
> > +    MIPSInsn m_opc_f = use_mips32r6_instructions ? OPC_SELEQZ : OPC_MOVZ;
> > +    const MIPSInsn m_opc_t_inv = m_opc_f;
> > +    const MIPSInsn m_opc_f_inv = m_opc_t;
> > +
> > +    /* If one of the values is zero, put it last to match SEL*Z 
> > instructions */
> > +    if (use_mips32r6_instructions && v1 == 0) {
> > +        v1 = v2;
> > +        v2 = 0;
> > +        cond = tcg_invert_cond(cond);
> > +    }
> >
> >       switch (cond) {
> >       case TCG_COND_EQ:
> > -        m_opc = OPC_MOVZ;
> > +        m_opc_t = m_opc_t_inv;
> > +        m_opc_f = m_opc_f_inv;
> >           /* FALLTHRU */
> >       case TCG_COND_NE:
> >           if (c2 != 0) {
> > @@ -877,14 +890,25 @@ static void tcg_out_movcond(TCGContext *s, TCGCond 
> > cond, TCGReg ret,
> >           /* Minimize code size by preferring a compare not requiring INV.  
> > */
> >           if (mips_cmp_map[cond] & MIPS_CMP_INV) {
> >               cond = tcg_invert_cond(cond);
> > -            m_opc = OPC_MOVZ;
> > +            m_opc_t = m_opc_t_inv;
> > +            m_opc_f = m_opc_f_inv;
> >           }
> >           tcg_out_setcond(s, cond, TCG_TMP0, c1, c2);
> >           c1 = TCG_TMP0;
> >           break;
> >       }
> >
> > -    tcg_out_opc_reg(s, m_opc, ret, v, c1);
> > +    if (use_mips32r6_instructions) {
> > +        if (v2 != 0) {
> > +            tcg_out_opc_reg(s, m_opc_f, TCG_TMP1, v2, c1);
> > +        }
> > +        tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
> > +        if (v2 != 0) {
> > +            tcg_out_opc_reg(s, OPC_OR, ret, ret, TCG_TMP1);
> > +        }
> > +    } else {
> > +        tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
> > +    }
> >   }
> 
> I think it's worth just using on "bool ne" in the generic code.  Then when 
> you 
> get down to the r6 conditional selecting the insns.
> 
> E.g.
> 
>      if (use_mips32r6_instructions) {
>          MIPSInsn m_opc_t = ne ? OPC_SELNEZ : OPC_SELEQZ;
>          MIPSInsn m_opf_f = ne ? OPC_SELEQZ : OPC_SELNEZ;
>          ...
>      } else {
>          MIPSInsn m_opc = ne ? OPC_MOVN : OPC_MOVZ;
>          ...
>      }
> 
> And maybe add either a tcg_debug_assert or a comment in the !r6 path to 
> remind 
> the reader that we took care of ret == v2 via constraints.

Okay, will do.

Thanks,
James

signature.asc
Description: Digital signature