diff mbox

[SPARC] Fix handling of conditional branches in delay slot of a conditional branch

Message ID d872097302eecfeea8770558857e2c2c3387464f.1312642248.git.atar4qemu@gmail.com
State New
Headers show

Commit Message

Artyom Tarasenko Aug. 6, 2011, 3:01 p.m. UTC
Check whether dc->npc is dynamic before using its value for branch.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
Particaluary the patch fixes handling of the constructions like

0x13e26c0:  brz,pn   %o0, 0x13e26e4
0x13e26c4:  brlez,pn   %o1, 0x13e26e4

present in NetBSD-5.1

 target-sparc/translate.c |   30 +++++++++++++++++++++---------
 1 files changed, 21 insertions(+), 9 deletions(-)

Comments

Blue Swirl Aug. 6, 2011, 8:14 p.m. UTC | #1
Thanks, applied.

On Sat, Aug 6, 2011 at 3:01 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> Check whether dc->npc is dynamic before using its value for branch.
>
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
> Particaluary the patch fixes handling of the constructions like
>
> 0x13e26c0:  brz,pn   %o0, 0x13e26e4
> 0x13e26c4:  brlez,pn   %o1, 0x13e26e4
>
> present in NetBSD-5.1
>
>  target-sparc/translate.c |   30 +++++++++++++++++++++---------
>  1 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 958fbc5..dee67b3 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond, TCGv r_src)
>  }
>  #endif
>
> -/* XXX: potentially incorrect if dynamic npc */
>  static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>                       TCGv r_cond)
>  {
> @@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>         } else {
>             dc->pc = dc->npc;
>             dc->jump_pc[0] = target;
> -            dc->jump_pc[1] = dc->npc + 4;
> -            dc->npc = JUMP_PC;
> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
> +                dc->jump_pc[1] = DYNAMIC_PC;
> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> +            } else {
> +                dc->jump_pc[1] = dc->npc + 4;
> +                dc->npc = JUMP_PC;
> +            }
>         }
>     }
>  }
>
> -/* XXX: potentially incorrect if dynamic npc */
>  static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>                       TCGv r_cond)
>  {
> @@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>         } else {
>             dc->pc = dc->npc;
>             dc->jump_pc[0] = target;
> -            dc->jump_pc[1] = dc->npc + 4;
> -            dc->npc = JUMP_PC;
> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
> +                dc->jump_pc[1] = DYNAMIC_PC;
> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> +            } else {
> +                dc->jump_pc[1] = dc->npc + 4;
> +                dc->npc = JUMP_PC;
> +            }
>         }
>     }
>  }
>
>  #ifdef TARGET_SPARC64
> -/* XXX: potentially incorrect if dynamic npc */
>  static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>                           TCGv r_cond, TCGv r_reg)
>  {
> @@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>     } else {
>         dc->pc = dc->npc;
>         dc->jump_pc[0] = target;
> -        dc->jump_pc[1] = dc->npc + 4;
> -        dc->npc = JUMP_PC;
> +        if (unlikely(dc->npc == DYNAMIC_PC)) {
> +            dc->jump_pc[1] = DYNAMIC_PC;
> +            tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> +        } else {
> +            dc->jump_pc[1] = dc->npc + 4;
> +            dc->npc = JUMP_PC;
> +        }
>     }
>  }
>
> --
> 1.7.3.4
>
>
Artyom Tarasenko Aug. 6, 2011, 9:33 p.m. UTC | #2
Since it's a pure bug fix, do you think can it be applied to 0.15 as well?

On Sat, Aug 6, 2011 at 10:14 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> Thanks, applied.
>
> On Sat, Aug 6, 2011 at 3:01 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>> Check whether dc->npc is dynamic before using its value for branch.
>>
>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> ---
>> Particaluary the patch fixes handling of the constructions like
>>
>> 0x13e26c0:  brz,pn   %o0, 0x13e26e4
>> 0x13e26c4:  brlez,pn   %o1, 0x13e26e4
>>
>> present in NetBSD-5.1
>>
>>  target-sparc/translate.c |   30 +++++++++++++++++++++---------
>>  1 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>> index 958fbc5..dee67b3 100644
>> --- a/target-sparc/translate.c
>> +++ b/target-sparc/translate.c
>> @@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond, TCGv r_src)
>>  }
>>  #endif
>>
>> -/* XXX: potentially incorrect if dynamic npc */
>>  static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>                       TCGv r_cond)
>>  {
>> @@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>         } else {
>>             dc->pc = dc->npc;
>>             dc->jump_pc[0] = target;
>> -            dc->jump_pc[1] = dc->npc + 4;
>> -            dc->npc = JUMP_PC;
>> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
>> +                dc->jump_pc[1] = DYNAMIC_PC;
>> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> +            } else {
>> +                dc->jump_pc[1] = dc->npc + 4;
>> +                dc->npc = JUMP_PC;
>> +            }
>>         }
>>     }
>>  }
>>
>> -/* XXX: potentially incorrect if dynamic npc */
>>  static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>                       TCGv r_cond)
>>  {
>> @@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>         } else {
>>             dc->pc = dc->npc;
>>             dc->jump_pc[0] = target;
>> -            dc->jump_pc[1] = dc->npc + 4;
>> -            dc->npc = JUMP_PC;
>> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
>> +                dc->jump_pc[1] = DYNAMIC_PC;
>> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> +            } else {
>> +                dc->jump_pc[1] = dc->npc + 4;
>> +                dc->npc = JUMP_PC;
>> +            }
>>         }
>>     }
>>  }
>>
>>  #ifdef TARGET_SPARC64
>> -/* XXX: potentially incorrect if dynamic npc */
>>  static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>>                           TCGv r_cond, TCGv r_reg)
>>  {
>> @@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>>     } else {
>>         dc->pc = dc->npc;
>>         dc->jump_pc[0] = target;
>> -        dc->jump_pc[1] = dc->npc + 4;
>> -        dc->npc = JUMP_PC;
>> +        if (unlikely(dc->npc == DYNAMIC_PC)) {
>> +            dc->jump_pc[1] = DYNAMIC_PC;
>> +            tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> +        } else {
>> +            dc->jump_pc[1] = dc->npc + 4;
>> +            dc->npc = JUMP_PC;
>> +        }
>>     }
>>  }
>>
>> --
>> 1.7.3.4
>>
>>
>
Blue Swirl Aug. 7, 2011, 9:25 a.m. UTC | #3
On Sat, Aug 6, 2011 at 9:33 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
> Since it's a pure bug fix, do you think can it be applied to 0.15 as well?

Maybe. Anthony/Jordan, please consider applying these to stable:
548f66d Fix handling of conditional branches in delay slot of a
conditional branch
6749432 Sparc: fix non-faulting unassigned memory accesses
ccb57e0 SPARC64: fix fnor* and fnand*

> On Sat, Aug 6, 2011 at 10:14 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> Thanks, applied.
>>
>> On Sat, Aug 6, 2011 at 3:01 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote:
>>> Check whether dc->npc is dynamic before using its value for branch.
>>>
>>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>> ---
>>> Particaluary the patch fixes handling of the constructions like
>>>
>>> 0x13e26c0:  brz,pn   %o0, 0x13e26e4
>>> 0x13e26c4:  brlez,pn   %o1, 0x13e26e4
>>>
>>> present in NetBSD-5.1
>>>
>>>  target-sparc/translate.c |   30 +++++++++++++++++++++---------
>>>  1 files changed, 21 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>>> index 958fbc5..dee67b3 100644
>>> --- a/target-sparc/translate.c
>>> +++ b/target-sparc/translate.c
>>> @@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond, TCGv r_src)
>>>  }
>>>  #endif
>>>
>>> -/* XXX: potentially incorrect if dynamic npc */
>>>  static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>>                       TCGv r_cond)
>>>  {
>>> @@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>>         } else {
>>>             dc->pc = dc->npc;
>>>             dc->jump_pc[0] = target;
>>> -            dc->jump_pc[1] = dc->npc + 4;
>>> -            dc->npc = JUMP_PC;
>>> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
>>> +                dc->jump_pc[1] = DYNAMIC_PC;
>>> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>>> +            } else {
>>> +                dc->jump_pc[1] = dc->npc + 4;
>>> +                dc->npc = JUMP_PC;
>>> +            }
>>>         }
>>>     }
>>>  }
>>>
>>> -/* XXX: potentially incorrect if dynamic npc */
>>>  static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>>                       TCGv r_cond)
>>>  {
>>> @@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
>>>         } else {
>>>             dc->pc = dc->npc;
>>>             dc->jump_pc[0] = target;
>>> -            dc->jump_pc[1] = dc->npc + 4;
>>> -            dc->npc = JUMP_PC;
>>> +            if (unlikely(dc->npc == DYNAMIC_PC)) {
>>> +                dc->jump_pc[1] = DYNAMIC_PC;
>>> +                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>>> +            } else {
>>> +                dc->jump_pc[1] = dc->npc + 4;
>>> +                dc->npc = JUMP_PC;
>>> +            }
>>>         }
>>>     }
>>>  }
>>>
>>>  #ifdef TARGET_SPARC64
>>> -/* XXX: potentially incorrect if dynamic npc */
>>>  static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>>>                           TCGv r_cond, TCGv r_reg)
>>>  {
>>> @@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>>>     } else {
>>>         dc->pc = dc->npc;
>>>         dc->jump_pc[0] = target;
>>> -        dc->jump_pc[1] = dc->npc + 4;
>>> -        dc->npc = JUMP_PC;
>>> +        if (unlikely(dc->npc == DYNAMIC_PC)) {
>>> +            dc->jump_pc[1] = DYNAMIC_PC;
>>> +            tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>>> +        } else {
>>> +            dc->jump_pc[1] = dc->npc + 4;
>>> +            dc->npc = JUMP_PC;
>>> +        }
>>>     }
>>>  }
>>>
>>> --
>>> 1.7.3.4
>>>
>>>
>>
>
>
>
> --
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>
diff mbox

Patch

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 958fbc5..dee67b3 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -1286,7 +1286,6 @@  static inline void gen_cond_reg(TCGv r_dst, int cond, TCGv r_src)
 }
 #endif
 
-/* XXX: potentially incorrect if dynamic npc */
 static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
                       TCGv r_cond)
 {
@@ -1321,13 +1320,17 @@  static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
         } else {
             dc->pc = dc->npc;
             dc->jump_pc[0] = target;
-            dc->jump_pc[1] = dc->npc + 4;
-            dc->npc = JUMP_PC;
+            if (unlikely(dc->npc == DYNAMIC_PC)) {
+                dc->jump_pc[1] = DYNAMIC_PC;
+                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+            } else {
+                dc->jump_pc[1] = dc->npc + 4;
+                dc->npc = JUMP_PC;
+            }
         }
     }
 }
 
-/* XXX: potentially incorrect if dynamic npc */
 static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
                       TCGv r_cond)
 {
@@ -1362,14 +1365,18 @@  static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
         } else {
             dc->pc = dc->npc;
             dc->jump_pc[0] = target;
-            dc->jump_pc[1] = dc->npc + 4;
-            dc->npc = JUMP_PC;
+            if (unlikely(dc->npc == DYNAMIC_PC)) {
+                dc->jump_pc[1] = DYNAMIC_PC;
+                tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+            } else {
+                dc->jump_pc[1] = dc->npc + 4;
+                dc->npc = JUMP_PC;
+            }
         }
     }
 }
 
 #ifdef TARGET_SPARC64
-/* XXX: potentially incorrect if dynamic npc */
 static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
                           TCGv r_cond, TCGv r_reg)
 {
@@ -1384,8 +1391,13 @@  static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
     } else {
         dc->pc = dc->npc;
         dc->jump_pc[0] = target;
-        dc->jump_pc[1] = dc->npc + 4;
-        dc->npc = JUMP_PC;
+        if (unlikely(dc->npc == DYNAMIC_PC)) {
+            dc->jump_pc[1] = DYNAMIC_PC;
+            tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+        } else {
+            dc->jump_pc[1] = dc->npc + 4;
+            dc->npc = JUMP_PC;
+        }
     }
 }