[v3,36/38] target-microblaze: Use tcg_gen_movcond in eval_cond_jmp

Message ID 20180516185146.30708-37-edgar.iglesias@gmail.com
State New
Headers show
Series
  • target-microblaze: Add support for Extended Addressing
Related show

Commit Message

Edgar E. Iglesias May 16, 2018, 6:51 p.m.
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(-)

Comments

Philippe Mathieu-Daudé May 17, 2018, 2:22 p.m. | #1
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)
>
Philippe Mathieu-Daudé May 17, 2018, 2:48 p.m. | #2
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)
>>
Edgar E. Iglesias May 17, 2018, 4:42 p.m. | #3
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)
> >>
Philippe Mathieu-Daudé May 17, 2018, 5:24 p.m. | #4
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)
>>>>
>
Richard Henderson May 17, 2018, 5:36 p.m. | #5
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~

Patch

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)