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

[Richard Henderson <rth@twiddle.net>]

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 <james.hogan@imgtec.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> 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.


r~