* [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32
@ 2012-09-26 18:48 Peter Maydell
2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Peter Maydell @ 2012-09-26 18:48 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Aurelien Jarno, Richard Henderson
These patches implement movcond_i32 for the ARM TCG backend; we
emit "mov dst, v2; cmp c1, c2; movcc dst, v1". We could have
done this with a pair of conditional movs, but (a) this is not
actually any shorter (b) it means we don't get the common TCG
code doing the work of avoiding "mov reg to itself" (c) conditional
moves aren't quite as free as they used to be on the ARM7.
(Tested using Aurelien's movcond-shift patches on the ARM frontend
as something that will generate enough movconds.)
Peter Maydell (2):
tcg/arm: Factor out code to emit immediate or reg-reg op
tcg/arm: Implement movcond_i32
tcg/arm/tcg-target.c | 56 +++++++++++++++++++++++++++-----------------------
tcg/arm/tcg-target.h | 2 +-
2 files changed, 31 insertions(+), 27 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op 2012-09-26 18:48 [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 Peter Maydell @ 2012-09-26 18:48 ` Peter Maydell 2012-09-26 19:01 ` Richard Henderson 2012-09-27 19:52 ` Aurelien Jarno 2012-09-26 18:48 ` [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 Peter Maydell ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Peter Maydell @ 2012-09-26 18:48 UTC (permalink / raw) To: qemu-devel; +Cc: patches, Aurelien Jarno, Richard Henderson The code to emit either an immediate cmp or a register cmp insn is duplicated in several places; factor it out into its own function. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tcg/arm/tcg-target.c | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c index 2bad0a2..a83b295 100644 --- a/tcg/arm/tcg-target.c +++ b/tcg/arm/tcg-target.c @@ -467,6 +467,21 @@ static inline void tcg_out_movi32(TCGContext *s, } } +static inline void tcg_out_dat_rI(TCGContext *s, int cond, int opc, TCGArg dst, + TCGArg lhs, TCGArg rhs, int rhs_is_const) +{ + /* Emit either the reg,imm or reg,reg form of a data-processing insn. + * rhs must satisfy the "rI" constraint. + */ + if (rhs_is_const) { + int rot = encode_imm(rhs); + assert(rot >= 0); + tcg_out_dat_imm(s, cond, opc, dst, lhs, rotl(rhs, rot) | (rot << 7)); + } else { + tcg_out_dat_reg(s, cond, opc, dst, lhs, rhs, SHIFT_IMM_LSL(0)); + } +} + static inline void tcg_out_mul32(TCGContext *s, int cond, int rd, int rs, int rm) { @@ -1591,14 +1606,7 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, c = ARITH_EOR; /* Fall through. */ gen_arith: - if (const_args[2]) { - int rot; - rot = encode_imm(args[2]); - tcg_out_dat_imm(s, COND_AL, c, - args[0], args[1], rotl(args[2], rot) | (rot << 7)); - } else - tcg_out_dat_reg(s, COND_AL, c, - args[0], args[1], args[2], SHIFT_IMM_LSL(0)); + tcg_out_dat_rI(s, COND_AL, c, args[0], args[1], args[2], const_args[2]); break; case INDEX_op_add2_i32: tcg_out_dat_reg2(s, COND_AL, ARITH_ADD, ARITH_ADC, @@ -1658,15 +1666,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, break; case INDEX_op_brcond_i32: - if (const_args[1]) { - int rot; - rot = encode_imm(args[1]); - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, - args[0], rotl(args[1], rot) | (rot << 7)); - } else { - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, - args[0], args[1], SHIFT_IMM_LSL(0)); - } + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, + args[0], args[1], const_args[1]); tcg_out_goto_label(s, tcg_cond_to_arm_cond[args[2]], args[3]); break; case INDEX_op_brcond2_i32: @@ -1685,15 +1686,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_goto_label(s, tcg_cond_to_arm_cond[args[4]], args[5]); break; case INDEX_op_setcond_i32: - if (const_args[2]) { - int rot; - rot = encode_imm(args[2]); - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, - args[1], rotl(args[2], rot) | (rot << 7)); - } else { - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, - args[1], args[2], SHIFT_IMM_LSL(0)); - } + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, + args[1], args[2], const_args[2]); tcg_out_dat_imm(s, tcg_cond_to_arm_cond[args[3]], ARITH_MOV, args[0], 0, 1); tcg_out_dat_imm(s, tcg_cond_to_arm_cond[tcg_invert_cond(args[3])], -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op 2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell @ 2012-09-26 19:01 ` Richard Henderson 2012-09-26 19:46 ` Peter Maydell 2012-09-27 19:52 ` Aurelien Jarno 1 sibling, 1 reply; 12+ messages in thread From: Richard Henderson @ 2012-09-26 19:01 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-devel, Aurelien Jarno, patches On 09/26/2012 11:48 AM, Peter Maydell wrote: > case INDEX_op_setcond_i32: > - if (const_args[2]) { > - int rot; > - rot = encode_imm(args[2]); > - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, > - args[1], rotl(args[2], rot) | (rot << 7)); > - } else { > - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, > - args[1], args[2], SHIFT_IMM_LSL(0)); > - } > + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, > + args[1], args[2], const_args[2]); > tcg_out_dat_imm(s, tcg_cond_to_arm_cond[args[3]], > ARITH_MOV, args[0], 0, 1); > tcg_out_dat_imm(s, tcg_cond_to_arm_cond[tcg_invert_cond(args[3])], The patch itself is fine. But as a followup, if movcc is no longer "free", then perhaps the setcond sequence is better as cmp mov movcc i.e. the second move is unconditional? A register renaming OOO core could then schedule the mov before the cmp. r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op 2012-09-26 19:01 ` Richard Henderson @ 2012-09-26 19:46 ` Peter Maydell 2012-09-27 13:01 ` Peter Maydell 0 siblings, 1 reply; 12+ messages in thread From: Peter Maydell @ 2012-09-26 19:46 UTC (permalink / raw) To: Richard Henderson; +Cc: qemu-devel, Aurelien Jarno, patches On 26 September 2012 20:01, Richard Henderson <rth@twiddle.net> wrote: > On 09/26/2012 11:48 AM, Peter Maydell wrote: >> case INDEX_op_setcond_i32: >> - if (const_args[2]) { >> - int rot; >> - rot = encode_imm(args[2]); >> - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, >> - args[1], rotl(args[2], rot) | (rot << 7)); >> - } else { >> - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, >> - args[1], args[2], SHIFT_IMM_LSL(0)); >> - } >> + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, >> + args[1], args[2], const_args[2]); >> tcg_out_dat_imm(s, tcg_cond_to_arm_cond[args[3]], >> ARITH_MOV, args[0], 0, 1); >> tcg_out_dat_imm(s, tcg_cond_to_arm_cond[tcg_invert_cond(args[3])], > > The patch itself is fine. But as a followup, if movcc is no longer "free", > then perhaps the setcond sequence is better as > > cmp > mov > movcc > > i.e. the second move is unconditional? A register renaming OOO core could > then schedule the mov before the cmp. Well, maybe. But my bar for changing existing code requires more proof that it's worth making the change (basically in both cases I'm optimising for convenience of development). My suspicion (entirely unbenchmarked) is that at the moment both these sequences will pan out about the same cost, so we might as well pick the one that's easiest to code. -- PMM ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op 2012-09-26 19:46 ` Peter Maydell @ 2012-09-27 13:01 ` Peter Maydell 0 siblings, 0 replies; 12+ messages in thread From: Peter Maydell @ 2012-09-27 13:01 UTC (permalink / raw) To: Richard Henderson; +Cc: qemu-devel, Aurelien Jarno, patches On 26 September 2012 20:46, Peter Maydell <peter.maydell@linaro.org> wrote: > On 26 September 2012 20:01, Richard Henderson <rth@twiddle.net> wrote: >> The patch itself is fine. But as a followup, if movcc is no longer "free", >> then perhaps the setcond sequence is better as >> >> cmp >> mov >> movcc >> >> i.e. the second move is unconditional? A register renaming OOO core could >> then schedule the mov before the cmp. > > Well, maybe. But my bar for changing existing code requires more > proof that it's worth making the change (basically in both cases > I'm optimising for convenience of development). My suspicion > (entirely unbenchmarked) is that at the moment both these sequences > will pan out about the same cost, so we might as well pick the > one that's easiest to code. Apparently there is really very little in it but if we did change then the recommendation would be to try something like: MOV dest, #0 CMP c1, c2 ADDcc dest, dest, #1 I could tell you why, but then I'd have to kill you :-) -- PMM ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op 2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell 2012-09-26 19:01 ` Richard Henderson @ 2012-09-27 19:52 ` Aurelien Jarno 1 sibling, 0 replies; 12+ messages in thread From: Aurelien Jarno @ 2012-09-27 19:52 UTC (permalink / raw) To: Peter Maydell; +Cc: Richard Henderson, qemu-devel, patches On Wed, Sep 26, 2012 at 07:48:54PM +0100, Peter Maydell wrote: > The code to emit either an immediate cmp or a register cmp insn is > duplicated in several places; factor it out into its own function. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > tcg/arm/tcg-target.c | 46 ++++++++++++++++++++-------------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) > > diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c > index 2bad0a2..a83b295 100644 > --- a/tcg/arm/tcg-target.c > +++ b/tcg/arm/tcg-target.c > @@ -467,6 +467,21 @@ static inline void tcg_out_movi32(TCGContext *s, > } > } > > +static inline void tcg_out_dat_rI(TCGContext *s, int cond, int opc, TCGArg dst, > + TCGArg lhs, TCGArg rhs, int rhs_is_const) > +{ > + /* Emit either the reg,imm or reg,reg form of a data-processing insn. > + * rhs must satisfy the "rI" constraint. > + */ > + if (rhs_is_const) { > + int rot = encode_imm(rhs); > + assert(rot >= 0); > + tcg_out_dat_imm(s, cond, opc, dst, lhs, rotl(rhs, rot) | (rot << 7)); > + } else { > + tcg_out_dat_reg(s, cond, opc, dst, lhs, rhs, SHIFT_IMM_LSL(0)); > + } > +} > + > static inline void tcg_out_mul32(TCGContext *s, > int cond, int rd, int rs, int rm) > { > @@ -1591,14 +1606,7 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > c = ARITH_EOR; > /* Fall through. */ > gen_arith: > - if (const_args[2]) { > - int rot; > - rot = encode_imm(args[2]); > - tcg_out_dat_imm(s, COND_AL, c, > - args[0], args[1], rotl(args[2], rot) | (rot << 7)); > - } else > - tcg_out_dat_reg(s, COND_AL, c, > - args[0], args[1], args[2], SHIFT_IMM_LSL(0)); > + tcg_out_dat_rI(s, COND_AL, c, args[0], args[1], args[2], const_args[2]); > break; > case INDEX_op_add2_i32: > tcg_out_dat_reg2(s, COND_AL, ARITH_ADD, ARITH_ADC, > @@ -1658,15 +1666,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > break; > > case INDEX_op_brcond_i32: > - if (const_args[1]) { > - int rot; > - rot = encode_imm(args[1]); > - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, > - args[0], rotl(args[1], rot) | (rot << 7)); > - } else { > - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, > - args[0], args[1], SHIFT_IMM_LSL(0)); > - } > + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, > + args[0], args[1], const_args[1]); > tcg_out_goto_label(s, tcg_cond_to_arm_cond[args[2]], args[3]); > break; > case INDEX_op_brcond2_i32: > @@ -1685,15 +1686,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > tcg_out_goto_label(s, tcg_cond_to_arm_cond[args[4]], args[5]); > break; > case INDEX_op_setcond_i32: > - if (const_args[2]) { > - int rot; > - rot = encode_imm(args[2]); > - tcg_out_dat_imm(s, COND_AL, ARITH_CMP, 0, > - args[1], rotl(args[2], rot) | (rot << 7)); > - } else { > - tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, > - args[1], args[2], SHIFT_IMM_LSL(0)); > - } > + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, > + args[1], args[2], const_args[2]); > tcg_out_dat_imm(s, tcg_cond_to_arm_cond[args[3]], > ARITH_MOV, args[0], 0, 1); > tcg_out_dat_imm(s, tcg_cond_to_arm_cond[tcg_invert_cond(args[3])], Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 2012-09-26 18:48 [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 Peter Maydell 2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell @ 2012-09-26 18:48 ` Peter Maydell 2012-09-27 6:33 ` Paolo Bonzini 2012-09-27 19:53 ` Aurelien Jarno 2012-10-12 17:07 ` [Qemu-devel] [PATCH 0/2] " Peter Maydell 2012-10-16 23:24 ` Aurelien Jarno 3 siblings, 2 replies; 12+ messages in thread From: Peter Maydell @ 2012-09-26 18:48 UTC (permalink / raw) To: qemu-devel; +Cc: patches, Aurelien Jarno, Richard Henderson Implement movcond_i32 for ARM, as the sequence mov dst, v2 (implicitly done by the tcg common code) cmp c1, c2 movCC dst, v1 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tcg/arm/tcg-target.c | 10 ++++++++++ tcg/arm/tcg-target.h | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c index a83b295..e38fd65 100644 --- a/tcg/arm/tcg-target.c +++ b/tcg/arm/tcg-target.c @@ -1587,6 +1587,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, case INDEX_op_movi_i32: tcg_out_movi32(s, COND_AL, args[0], args[1]); break; + case INDEX_op_movcond_i32: + /* Constraints mean that v2 is always in the same register as dest, + * so we only need to do "if condition passed, move v1 to dest". + */ + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, + args[1], args[2], const_args[2]); + tcg_out_dat_rI(s, tcg_cond_to_arm_cond[args[5]], + ARITH_MOV, args[0], 0, args[3], const_args[3]); + break; case INDEX_op_add_i32: c = ARITH_ADD; goto gen_arith; @@ -1798,6 +1807,7 @@ static const TCGTargetOpDef arm_op_defs[] = { { INDEX_op_brcond_i32, { "r", "rI" } }, { INDEX_op_setcond_i32, { "r", "r", "rI" } }, + { INDEX_op_movcond_i32, { "r", "r", "rI", "rI", "0" } }, /* TODO: "r", "r", "r", "r", "ri", "ri" */ { INDEX_op_add2_i32, { "r", "r", "r", "r", "r", "r" } }, diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h index e2299ca..0df3352 100644 --- a/tcg/arm/tcg-target.h +++ b/tcg/arm/tcg-target.h @@ -73,7 +73,7 @@ typedef enum { #define TCG_TARGET_HAS_nand_i32 0 #define TCG_TARGET_HAS_nor_i32 0 #define TCG_TARGET_HAS_deposit_i32 0 -#define TCG_TARGET_HAS_movcond_i32 0 +#define TCG_TARGET_HAS_movcond_i32 1 #define TCG_TARGET_HAS_GUEST_BASE -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 2012-09-26 18:48 ` [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 Peter Maydell @ 2012-09-27 6:33 ` Paolo Bonzini 2012-09-27 14:32 ` Richard Henderson 2012-09-27 19:53 ` Aurelien Jarno 1 sibling, 1 reply; 12+ messages in thread From: Paolo Bonzini @ 2012-09-27 6:33 UTC (permalink / raw) To: Peter Maydell; +Cc: Richard Henderson, qemu-devel, Aurelien Jarno, patches Il 26/09/2012 20:48, Peter Maydell ha scritto: > Implement movcond_i32 for ARM, as the sequence > mov dst, v2 (implicitly done by the tcg common code) > cmp c1, c2 > movCC dst, v1 Should you make tcg/optimize.c prefer "movcond a, a, b" to "movcond a, b, a", similar to commit c2b0e2f (tcg/optimize: prefer the "op a, a, b" form for commutative ops, 2012-09-19)? Paolo > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > tcg/arm/tcg-target.c | 10 ++++++++++ > tcg/arm/tcg-target.h | 2 +- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c > index a83b295..e38fd65 100644 > --- a/tcg/arm/tcg-target.c > +++ b/tcg/arm/tcg-target.c > @@ -1587,6 +1587,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > case INDEX_op_movi_i32: > tcg_out_movi32(s, COND_AL, args[0], args[1]); > break; > + case INDEX_op_movcond_i32: > + /* Constraints mean that v2 is always in the same register as dest, > + * so we only need to do "if condition passed, move v1 to dest". > + */ > + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, > + args[1], args[2], const_args[2]); > + tcg_out_dat_rI(s, tcg_cond_to_arm_cond[args[5]], > + ARITH_MOV, args[0], 0, args[3], const_args[3]); > + break; > case INDEX_op_add_i32: > c = ARITH_ADD; > goto gen_arith; > @@ -1798,6 +1807,7 @@ static const TCGTargetOpDef arm_op_defs[] = { > > { INDEX_op_brcond_i32, { "r", "rI" } }, > { INDEX_op_setcond_i32, { "r", "r", "rI" } }, > + { INDEX_op_movcond_i32, { "r", "r", "rI", "rI", "0" } }, > > /* TODO: "r", "r", "r", "r", "ri", "ri" */ > { INDEX_op_add2_i32, { "r", "r", "r", "r", "r", "r" } }, > diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h > index e2299ca..0df3352 100644 > --- a/tcg/arm/tcg-target.h > +++ b/tcg/arm/tcg-target.h > @@ -73,7 +73,7 @@ typedef enum { > #define TCG_TARGET_HAS_nand_i32 0 > #define TCG_TARGET_HAS_nor_i32 0 > #define TCG_TARGET_HAS_deposit_i32 0 > -#define TCG_TARGET_HAS_movcond_i32 0 > +#define TCG_TARGET_HAS_movcond_i32 1 > > #define TCG_TARGET_HAS_GUEST_BASE > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 2012-09-27 6:33 ` Paolo Bonzini @ 2012-09-27 14:32 ` Richard Henderson 0 siblings, 0 replies; 12+ messages in thread From: Richard Henderson @ 2012-09-27 14:32 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, qemu-devel, Aurelien Jarno, patches On 09/26/2012 11:33 PM, Paolo Bonzini wrote: > Should you make tcg/optimize.c prefer "movcond a, a, b" to "movcond a, > b, a", similar to commit c2b0e2f (tcg/optimize: prefer the "op a, a, b" > form for commutative ops, 2012-09-19)? I explicitly went with "a, b, a" because I find it is easier to reason with positive logic (move-if-true) than negative logic. r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 2012-09-26 18:48 ` [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 Peter Maydell 2012-09-27 6:33 ` Paolo Bonzini @ 2012-09-27 19:53 ` Aurelien Jarno 1 sibling, 0 replies; 12+ messages in thread From: Aurelien Jarno @ 2012-09-27 19:53 UTC (permalink / raw) To: Peter Maydell; +Cc: Richard Henderson, qemu-devel, patches On Wed, Sep 26, 2012 at 07:48:55PM +0100, Peter Maydell wrote: > Implement movcond_i32 for ARM, as the sequence > mov dst, v2 (implicitly done by the tcg common code) > cmp c1, c2 > movCC dst, v1 > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > tcg/arm/tcg-target.c | 10 ++++++++++ > tcg/arm/tcg-target.h | 2 +- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c > index a83b295..e38fd65 100644 > --- a/tcg/arm/tcg-target.c > +++ b/tcg/arm/tcg-target.c > @@ -1587,6 +1587,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > case INDEX_op_movi_i32: > tcg_out_movi32(s, COND_AL, args[0], args[1]); > break; > + case INDEX_op_movcond_i32: > + /* Constraints mean that v2 is always in the same register as dest, > + * so we only need to do "if condition passed, move v1 to dest". > + */ > + tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0, > + args[1], args[2], const_args[2]); > + tcg_out_dat_rI(s, tcg_cond_to_arm_cond[args[5]], > + ARITH_MOV, args[0], 0, args[3], const_args[3]); > + break; > case INDEX_op_add_i32: > c = ARITH_ADD; > goto gen_arith; > @@ -1798,6 +1807,7 @@ static const TCGTargetOpDef arm_op_defs[] = { > > { INDEX_op_brcond_i32, { "r", "rI" } }, > { INDEX_op_setcond_i32, { "r", "r", "rI" } }, > + { INDEX_op_movcond_i32, { "r", "r", "rI", "rI", "0" } }, > > /* TODO: "r", "r", "r", "r", "ri", "ri" */ > { INDEX_op_add2_i32, { "r", "r", "r", "r", "r", "r" } }, > diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h > index e2299ca..0df3352 100644 > --- a/tcg/arm/tcg-target.h > +++ b/tcg/arm/tcg-target.h > @@ -73,7 +73,7 @@ typedef enum { > #define TCG_TARGET_HAS_nand_i32 0 > #define TCG_TARGET_HAS_nor_i32 0 > #define TCG_TARGET_HAS_deposit_i32 0 > -#define TCG_TARGET_HAS_movcond_i32 0 > +#define TCG_TARGET_HAS_movcond_i32 1 > > #define TCG_TARGET_HAS_GUEST_BASE > Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 2012-09-26 18:48 [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 Peter Maydell 2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell 2012-09-26 18:48 ` [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 Peter Maydell @ 2012-10-12 17:07 ` Peter Maydell 2012-10-16 23:24 ` Aurelien Jarno 3 siblings, 0 replies; 12+ messages in thread From: Peter Maydell @ 2012-10-12 17:07 UTC (permalink / raw) To: qemu-devel; +Cc: Richard Henderson, Aurelien Jarno, patches On 26 September 2012 19:48, Peter Maydell <peter.maydell@linaro.org> wrote: > These patches implement movcond_i32 for the ARM TCG backend; we > emit "mov dst, v2; cmp c1, c2; movcc dst, v1". We could have > done this with a pair of conditional movs, but (a) this is not > actually any shorter (b) it means we don't get the common TCG > code doing the work of avoiding "mov reg to itself" (c) conditional > moves aren't quite as free as they used to be on the ARM7. Ping? -- PMM ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 2012-09-26 18:48 [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 Peter Maydell ` (2 preceding siblings ...) 2012-10-12 17:07 ` [Qemu-devel] [PATCH 0/2] " Peter Maydell @ 2012-10-16 23:24 ` Aurelien Jarno 3 siblings, 0 replies; 12+ messages in thread From: Aurelien Jarno @ 2012-10-16 23:24 UTC (permalink / raw) To: Peter Maydell; +Cc: Richard Henderson, qemu-devel, patches On Wed, Sep 26, 2012 at 07:48:53PM +0100, Peter Maydell wrote: > These patches implement movcond_i32 for the ARM TCG backend; we > emit "mov dst, v2; cmp c1, c2; movcc dst, v1". We could have > done this with a pair of conditional movs, but (a) this is not > actually any shorter (b) it means we don't get the common TCG > code doing the work of avoiding "mov reg to itself" (c) conditional > moves aren't quite as free as they used to be on the ARM7. > > (Tested using Aurelien's movcond-shift patches on the ARM frontend > as something that will generate enough movconds.) > > Peter Maydell (2): > tcg/arm: Factor out code to emit immediate or reg-reg op > tcg/arm: Implement movcond_i32 > > tcg/arm/tcg-target.c | 56 +++++++++++++++++++++++++++----------------------- > tcg/arm/tcg-target.h | 2 +- > 2 files changed, 31 insertions(+), 27 deletions(-) > > -- > 1.7.9.5 > Thanks, both applied. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-10-16 23:24 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-26 18:48 [Qemu-devel] [PATCH 0/2] tcg/arm: Implement movcond_i32 Peter Maydell 2012-09-26 18:48 ` [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op Peter Maydell 2012-09-26 19:01 ` Richard Henderson 2012-09-26 19:46 ` Peter Maydell 2012-09-27 13:01 ` Peter Maydell 2012-09-27 19:52 ` Aurelien Jarno 2012-09-26 18:48 ` [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32 Peter Maydell 2012-09-27 6:33 ` Paolo Bonzini 2012-09-27 14:32 ` Richard Henderson 2012-09-27 19:53 ` Aurelien Jarno 2012-10-12 17:07 ` [Qemu-devel] [PATCH 0/2] " Peter Maydell 2012-10-16 23:24 ` Aurelien Jarno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).