| Message ID | 1443788657-14537-7-git-send-email-james.hogan@imgtec.com |
|---|---|
| State | New |
| Headers | show |
On 10/02/2015 10:24 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> Reviewed-by: Richard Henderson <rth@twiddle.net> > { INDEX_op_brcond_i32, { "rZ", "rZ" } }, > +#if !use_mips32r6_instructions > { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } }, > +#else > + { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } }, > +#endif The only thing I'd change is preferring positive tests to negative ones. So swap the order of these lines, and the sense of the #if. Leon, do you want to take this as a mips maintainer, or shall I as tcg maintainer? r~
On Wed, Oct 07, 2015 at 08:46:30PM +1100, Richard Henderson wrote: > On 10/02/2015 10:24 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> > > Reviewed-by: Richard Henderson <rth@twiddle.net> Thanks for the reviewing! > > > > { INDEX_op_brcond_i32, { "rZ", "rZ" } }, > > +#if !use_mips32r6_instructions > > { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } }, > > +#else > > + { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } }, > > +#endif > > > The only thing I'd change is preferring positive tests to negative ones. So > swap the order of these lines, and the sense of the #if. No problem. Shall I do a full resend for that, or can it be fixed up by whoever applies? Cheers James > > Leon, do you want to take this as a mips maintainer, or shall I as tcg maintainer? > > > r~
On 07/10/15 10:46, Richard Henderson wrote: > Leon, do you want to take this as a mips maintainer, or shall I as tcg > maintainer? I thought this would go via Aurelien's mips tcg-backend queue. But if Aurelien is busy, could you take them? (at the moment I don't have anything handy to test the mips backend). Thanks, Leon
On 10/07/2015 09:34 PM, James Hogan wrote: >>> { INDEX_op_brcond_i32, { "rZ", "rZ" } }, >>> +#if !use_mips32r6_instructions >>> { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } }, >>> +#else >>> + { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } }, >>> +#endif >> >> >> The only thing I'd change is preferring positive tests to negative ones. So >> swap the order of these lines, and the sense of the #if. > > No problem. Shall I do a full resend for that, or can it be fixed up by > whoever applies? No resend needed. I'll fix it when applying to my tcg queue. r~
On 2015-10-02 13:24, 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 v3: > - Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ > instead of SELNEZ / MOVN (Richard). > - Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind > reader that it should be guaranteed via constraints (Richard). > > 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 | 43 +++++++++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 6 deletions(-) Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
On 2015-10-07 12:47, Leon Alrae wrote: > On 07/10/15 10:46, Richard Henderson wrote: > > Leon, do you want to take this as a mips maintainer, or shall I as tcg > > maintainer? > > I thought this would go via Aurelien's mips tcg-backend queue. But if > Aurelien is busy, could you take them? (at the moment I don't have > anything handy to test the mips backend). Sorry I have been indeed a bit busy. I can send a pull request in the next days. As you prefer.
On Thu, Oct 08, 2015 at 06:31:32PM +0200, Aurelien Jarno wrote: > On 2015-10-02 13:24, 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 v3: > > - Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ > > instead of SELNEZ / MOVN (Richard). > > - Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind > > reader that it should be guaranteed via constraints (Richard). > > > > 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 | 43 +++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 37 insertions(+), 6 deletions(-) > > Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> Thanks for the reviews Aurelien! Cheers James
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c index a937b1475d04..eff498991096 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,20 @@ 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; + bool eqz = false; + + /* 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; + eqz = true; /* FALLTHRU */ case TCG_COND_NE: if (c2 != 0) { @@ -877,14 +886,32 @@ 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; + eqz = true; } 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) { + MIPSInsn m_opc_t = eqz ? OPC_SELEQZ : OPC_SELNEZ; + MIPSInsn m_opc_f = eqz ? OPC_SELNEZ : OPC_SELEQZ; + + 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 { + MIPSInsn m_opc = eqz ? OPC_MOVZ : OPC_MOVN; + + tcg_out_opc_reg(s, m_opc, ret, v1, c1); + + /* This should be guaranteed via constraints */ + tcg_debug_assert(v2 == ret); + } } static void tcg_out_call_int(TCGContext *s, tcg_insn_unit *arg, bool tail) @@ -1577,7 +1604,7 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, break; case INDEX_op_movcond_i32: - tcg_out_movcond(s, args[5], a0, a1, a2, args[3]); + tcg_out_movcond(s, args[5], a0, a1, a2, args[3], args[4]); break; case INDEX_op_setcond_i32: @@ -1666,7 +1693,11 @@ static const TCGTargetOpDef mips_op_defs[] = { { INDEX_op_deposit_i32, { "r", "0", "rZ" } }, { INDEX_op_brcond_i32, { "rZ", "rZ" } }, +#if !use_mips32r6_instructions { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } }, +#else + { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } }, +#endif { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } }, { INDEX_op_setcond2_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
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 v3: - Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ instead of SELNEZ / MOVN (Richard). - Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind reader that it should be guaranteed via constraints (Richard). 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 | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-)