| Message ID | 20180516185146.30708-37-edgar.iglesias@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target-microblaze: Add support for Extended Addressing | expand |
On 05/16/2018 03:51 PM, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Cleanup eval_cond_jmp to use tcg_gen_movcond_i64(). > No functional change. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target/microblaze/translate.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c > index a846797d9c..78c2855ff0 100644 > --- a/target/microblaze/translate.c > +++ b/target/microblaze/translate.c > @@ -1171,12 +1171,16 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc, > > static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false) > { > - TCGLabel *l1 = gen_new_label(); > - /* Conditional jmp. */ > - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false); > - tcg_gen_brcondi_i32(TCG_COND_EQ, env_btaken, 0, l1); > - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true); > - gen_set_label(l1); > + TCGv_i64 tmp_btaken = tcg_temp_new_i64(); > + TCGv_i64 tmp_zero = tcg_const_i64(0); > + > + tcg_gen_extu_i32_i64(tmp_btaken, env_btaken); env_btaken is i32, ok. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], > + tmp_btaken, tmp_zero, > + pc_true, pc_false); > + > + tcg_temp_free_i64(tmp_btaken); > + tcg_temp_free_i64(tmp_zero); > } > > static void dec_bcc(DisasContext *dc) >
Hi Edgar, On 05/17/2018 11:22 AM, Philippe Mathieu-Daudé wrote: > On 05/16/2018 03:51 PM, Edgar E. Iglesias wrote: >> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> >> >> Cleanup eval_cond_jmp to use tcg_gen_movcond_i64(). >> No functional change. >> >> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> >> --- >> target/microblaze/translate.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c >> index a846797d9c..78c2855ff0 100644 >> --- a/target/microblaze/translate.c >> +++ b/target/microblaze/translate.c >> @@ -1171,12 +1171,16 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc, >> >> static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false) >> { >> - TCGLabel *l1 = gen_new_label(); >> - /* Conditional jmp. */ >> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false); >> - tcg_gen_brcondi_i32(TCG_COND_EQ, env_btaken, 0, l1); >> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true); >> - gen_set_label(l1); >> + TCGv_i64 tmp_btaken = tcg_temp_new_i64(); >> + TCGv_i64 tmp_zero = tcg_const_i64(0); Why not use cpu_R[0] directly? I wonder, maybe the current model doesn't not have R0 hardwired to 0? >> + >> + tcg_gen_extu_i32_i64(tmp_btaken, env_btaken); > > env_btaken is i32, ok. > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> + tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], >> + tmp_btaken, tmp_zero, >> + pc_true, pc_false); So we could drop a temp, using: tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], tmp_btaken, cpu_R[0], pc_true, pc_false); >> + >> + tcg_temp_free_i64(tmp_btaken); >> + tcg_temp_free_i64(tmp_zero); >> } >> >> static void dec_bcc(DisasContext *dc) >>
On Thu, May 17, 2018 at 11:48:35AM -0300, Philippe Mathieu-Daudé wrote: > Hi Edgar, > > On 05/17/2018 11:22 AM, Philippe Mathieu-Daudé wrote: > > On 05/16/2018 03:51 PM, Edgar E. Iglesias wrote: > >> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > >> > >> Cleanup eval_cond_jmp to use tcg_gen_movcond_i64(). > >> No functional change. > >> > >> Suggested-by: Richard Henderson <richard.henderson@linaro.org> > >> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > >> --- > >> target/microblaze/translate.c | 16 ++++++++++------ > >> 1 file changed, 10 insertions(+), 6 deletions(-) > >> > >> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c > >> index a846797d9c..78c2855ff0 100644 > >> --- a/target/microblaze/translate.c > >> +++ b/target/microblaze/translate.c > >> @@ -1171,12 +1171,16 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc, > >> > >> static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false) > >> { > >> - TCGLabel *l1 = gen_new_label(); > >> - /* Conditional jmp. */ > >> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false); > >> - tcg_gen_brcondi_i32(TCG_COND_EQ, env_btaken, 0, l1); > >> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true); > >> - gen_set_label(l1); > >> + TCGv_i64 tmp_btaken = tcg_temp_new_i64(); > >> + TCGv_i64 tmp_zero = tcg_const_i64(0); > > Why not use cpu_R[0] directly? > > I wonder, maybe the current model doesn't not have R0 hardwired to 0? It does but cpu_R[0] is 32bit so we would still need a temp to extend it... Cheers, Edgar > > >> + > >> + tcg_gen_extu_i32_i64(tmp_btaken, env_btaken); > > > > env_btaken is i32, ok. > > > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > > >> + tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], > >> + tmp_btaken, tmp_zero, > >> + pc_true, pc_false); > > So we could drop a temp, using: > > tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], > tmp_btaken, cpu_R[0], > pc_true, pc_false); > > >> + > >> + tcg_temp_free_i64(tmp_btaken); > >> + tcg_temp_free_i64(tmp_zero); > >> } > >> > >> static void dec_bcc(DisasContext *dc) > >>
On 05/17/2018 01:42 PM, Edgar E. Iglesias wrote: > On Thu, May 17, 2018 at 11:48:35AM -0300, Philippe Mathieu-Daudé wrote: >> Hi Edgar, >> >> On 05/17/2018 11:22 AM, Philippe Mathieu-Daudé wrote: >>> On 05/16/2018 03:51 PM, Edgar E. Iglesias wrote: >>>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> >>>> >>>> Cleanup eval_cond_jmp to use tcg_gen_movcond_i64(). >>>> No functional change. >>>> >>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> >>>> --- >>>> target/microblaze/translate.c | 16 ++++++++++------ >>>> 1 file changed, 10 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c >>>> index a846797d9c..78c2855ff0 100644 >>>> --- a/target/microblaze/translate.c >>>> +++ b/target/microblaze/translate.c >>>> @@ -1171,12 +1171,16 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc, >>>> >>>> static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false) >>>> { >>>> - TCGLabel *l1 = gen_new_label(); >>>> - /* Conditional jmp. */ >>>> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false); >>>> - tcg_gen_brcondi_i32(TCG_COND_EQ, env_btaken, 0, l1); >>>> - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true); >>>> - gen_set_label(l1); >>>> + TCGv_i64 tmp_btaken = tcg_temp_new_i64(); >>>> + TCGv_i64 tmp_zero = tcg_const_i64(0); >> >> Why not use cpu_R[0] directly? >> >> I wonder, maybe the current model doesn't not have R0 hardwired to 0? > > It does but cpu_R[0] is 32bit so we would still need a temp to extend it... Oh true, I missed that. Thanks :) > > Cheers, > Edgar > >> >>>> + >>>> + tcg_gen_extu_i32_i64(tmp_btaken, env_btaken); >>> >>> env_btaken is i32, ok. >>> >>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> >>>> + tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], >>>> + tmp_btaken, tmp_zero, >>>> + pc_true, pc_false); >> >> So we could drop a temp, using: >> >> tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], >> tmp_btaken, cpu_R[0], >> pc_true, pc_false); >> >>>> + >>>> + tcg_temp_free_i64(tmp_btaken); >>>> + tcg_temp_free_i64(tmp_zero); >>>> } >>>> >>>> static void dec_bcc(DisasContext *dc) >>>> >
On 05/16/2018 11:51 AM, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Cleanup eval_cond_jmp to use tcg_gen_movcond_i64(). > No functional change. > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target/microblaze/translate.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c index a846797d9c..78c2855ff0 100644 --- a/target/microblaze/translate.c +++ b/target/microblaze/translate.c @@ -1171,12 +1171,16 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc, static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false) { - TCGLabel *l1 = gen_new_label(); - /* Conditional jmp. */ - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false); - tcg_gen_brcondi_i32(TCG_COND_EQ, env_btaken, 0, l1); - tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true); - gen_set_label(l1); + TCGv_i64 tmp_btaken = tcg_temp_new_i64(); + TCGv_i64 tmp_zero = tcg_const_i64(0); + + tcg_gen_extu_i32_i64(tmp_btaken, env_btaken); + tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC], + tmp_btaken, tmp_zero, + pc_true, pc_false); + + tcg_temp_free_i64(tmp_btaken); + tcg_temp_free_i64(tmp_zero); } static void dec_bcc(DisasContext *dc)