[2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0
diff mbox series

Message ID 20191117090621.32425-3-richard.henderson@linaro.org
State New
Headers show
Series
  • target/arm: two fixes for ldrex/strex
Related show

Commit Message

Richard Henderson Nov. 17, 2019, 9:06 a.m. UTC
Armv8-A removes UNPREDICTABLE for R13 for these cases.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Peter Maydell Nov. 18, 2019, 1:10 p.m. UTC | #1
On Sun, 17 Nov 2019 at 09:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Armv8-A removes UNPREDICTABLE for R13 for these cases.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index b285b23858..3db8103966 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8931,11 +8931,11 @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)
>      /* We UNDEF for these UNPREDICTABLE cases.  */
>      if (a->rd == 15 || a->rn == 15 || a->rt == 15
>          || a->rd == a->rn || a->rd == a->rt
> -        || (s->thumb && (a->rd == 13 || a->rt == 13))
> +        || (!ENABLE_ARCH_8 && s->thumb && (a->rd == 13 || a->rt == 13))
>          || (mop == MO_64
>              && (a->rt2 == 15
>                  || a->rd == a->rt2
> -                || (s->thumb && a->rt2 == 13)))) {
> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
>          unallocated_encoding(s);
>          return true;
>      }
> @@ -9087,10 +9087,10 @@ static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq)
>
>      /* We UNDEF for these UNPREDICTABLE cases.  */
>      if (a->rn == 15 || a->rt == 15
> -        || (s->thumb && a->rt == 13)
> +        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
>          || (mop == MO_64
>              && (a->rt2 == 15 || a->rt == a->rt2
> -                || (s->thumb && a->rt2 == 13)))) {
> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
>          unallocated_encoding(s);
>          return true;
>      }

These cases for r13 are indeed no longer UNPREDICTABLE in
v8A, but they are still marked as UNPREDICTABLE for v8M...

thanks
-- PMM
Richard Henderson Nov. 18, 2019, 1:15 p.m. UTC | #2
On 11/18/19 2:10 PM, Peter Maydell wrote:
>>      /* We UNDEF for these UNPREDICTABLE cases.  */
>>      if (a->rn == 15 || a->rt == 15
>> -        || (s->thumb && a->rt == 13)
>> +        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
>>          || (mop == MO_64
>>              && (a->rt2 == 15 || a->rt == a->rt2
>> -                || (s->thumb && a->rt2 == 13)))) {
>> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
>>          unallocated_encoding(s);
>>          return true;
>>      }
> 
> These cases for r13 are indeed no longer UNPREDICTABLE in
> v8A, but they are still marked as UNPREDICTABLE for v8M...

Ho hum.  I knew I should have looked at that doc as well...


r~
Peter Maydell Nov. 18, 2019, 5:53 p.m. UTC | #3
On Mon, 18 Nov 2019 at 13:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/18/19 2:10 PM, Peter Maydell wrote:
> >>      /* We UNDEF for these UNPREDICTABLE cases.  */
> >>      if (a->rn == 15 || a->rt == 15
> >> -        || (s->thumb && a->rt == 13)
> >> +        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
> >>          || (mop == MO_64
> >>              && (a->rt2 == 15 || a->rt == a->rt2
> >> -                || (s->thumb && a->rt2 == 13)))) {
> >> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
> >>          unallocated_encoding(s);
> >>          return true;
> >>      }
> >
> > These cases for r13 are indeed no longer UNPREDICTABLE in
> > v8A, but they are still marked as UNPREDICTABLE for v8M...
>
> Ho hum.  I knew I should have looked at that doc as well...

I would like to get this in for rc2 tomorrow, so I propose
to squash in changes to give the following result (basically
turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'):


diff --git a/target/arm/translate.c b/target/arm/translate.c
index b285b23858e..4d5d4bd8886 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a)
 static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)
 {
     TCGv_i32 addr;
+    /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */
+    bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);

     /* We UNDEF for these UNPREDICTABLE cases.  */
     if (a->rd == 15 || a->rn == 15 || a->rt == 15
         || a->rd == a->rn || a->rd == a->rt
-        || (s->thumb && (a->rd == 13 || a->rt == 13))
+        || (!v8a && s->thumb && (a->rd == 13 || a->rt == 13))
         || (mop == MO_64
             && (a->rt2 == 15
                 || a->rd == a->rt2
-                || (s->thumb && a->rt2 == 13)))) {
+                || (!v8a && s->thumb && a->rt2 == 13)))) {
         unallocated_encoding(s);
         return true;
     }
@@ -9084,13 +9086,15 @@ static bool trans_STLH(DisasContext *s, arg_STL *a)
 static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq)
 {
     TCGv_i32 addr;
+    /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */
+    bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);

     /* We UNDEF for these UNPREDICTABLE cases.  */
     if (a->rn == 15 || a->rt == 15
-        || (s->thumb && a->rt == 13)
+        || (!v8a && s->thumb && a->rt == 13)
         || (mop == MO_64
             && (a->rt2 == 15 || a->rt == a->rt2
-                || (s->thumb && a->rt2 == 13)))) {
+                || (!v8a && s->thumb && a->rt2 == 13)))) {
         unallocated_encoding(s);
         return true;
     }

thanks
-- PMM
Richard Henderson Nov. 18, 2019, 8:02 p.m. UTC | #4
On 11/18/19 6:53 PM, Peter Maydell wrote:
> On Mon, 18 Nov 2019 at 13:16, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 11/18/19 2:10 PM, Peter Maydell wrote:
>>>>      /* We UNDEF for these UNPREDICTABLE cases.  */
>>>>      if (a->rn == 15 || a->rt == 15
>>>> -        || (s->thumb && a->rt == 13)
>>>> +        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
>>>>          || (mop == MO_64
>>>>              && (a->rt2 == 15 || a->rt == a->rt2
>>>> -                || (s->thumb && a->rt2 == 13)))) {
>>>> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
>>>>          unallocated_encoding(s);
>>>>          return true;
>>>>      }
>>>
>>> These cases for r13 are indeed no longer UNPREDICTABLE in
>>> v8A, but they are still marked as UNPREDICTABLE for v8M...
>>
>> Ho hum.  I knew I should have looked at that doc as well...
> 
> I would like to get this in for rc2 tomorrow, so I propose
> to squash in changes to give the following result (basically
> turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'):
> 
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index b285b23858e..4d5d4bd8886 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a)
>  static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)
>  {
>      TCGv_i32 addr;
> +    /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */
> +    bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);

Sorry, I wrote the patch but got distracted with other bugs without getting
around to posting.  I had solved this with a new ENABLE_ARCH_8A, but this
version works for me as well.

Thanks,

r~
Peter Maydell Nov. 18, 2019, 9:22 p.m. UTC | #5
On Mon, 18 Nov 2019 at 20:02, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/18/19 6:53 PM, Peter Maydell wrote:
> > On Mon, 18 Nov 2019 at 13:16, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 11/18/19 2:10 PM, Peter Maydell wrote:
> >>>>      /* We UNDEF for these UNPREDICTABLE cases.  */
> >>>>      if (a->rn == 15 || a->rt == 15
> >>>> -        || (s->thumb && a->rt == 13)
> >>>> +        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
> >>>>          || (mop == MO_64
> >>>>              && (a->rt2 == 15 || a->rt == a->rt2
> >>>> -                || (s->thumb && a->rt2 == 13)))) {
> >>>> +                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
> >>>>          unallocated_encoding(s);
> >>>>          return true;
> >>>>      }
> >>>
> >>> These cases for r13 are indeed no longer UNPREDICTABLE in
> >>> v8A, but they are still marked as UNPREDICTABLE for v8M...
> >>
> >> Ho hum.  I knew I should have looked at that doc as well...
> >
> > I would like to get this in for rc2 tomorrow, so I propose
> > to squash in changes to give the following result (basically
> > turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'):
> >
> >
> > diff --git a/target/arm/translate.c b/target/arm/translate.c
> > index b285b23858e..4d5d4bd8886 100644
> > --- a/target/arm/translate.c
> > +++ b/target/arm/translate.c
> > @@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a)
> >  static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)
> >  {
> >      TCGv_i32 addr;
> > +    /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */
> > +    bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M);
>
> Sorry, I wrote the patch but got distracted with other bugs without getting
> around to posting.  I had solved this with a new ENABLE_ARCH_8A, but this
> version works for me as well.

At some point we should decide whether we prefer these ENABLE
macros or just to open-code arm_dc_feature() calls, because the
current mix is a bit odd... (for code I've written I've tended to the
arm_dc_feature() approach).

thanks
-- PMM

Patch
diff mbox series

diff --git a/target/arm/translate.c b/target/arm/translate.c
index b285b23858..3db8103966 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8931,11 +8931,11 @@  static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel)
     /* We UNDEF for these UNPREDICTABLE cases.  */
     if (a->rd == 15 || a->rn == 15 || a->rt == 15
         || a->rd == a->rn || a->rd == a->rt
-        || (s->thumb && (a->rd == 13 || a->rt == 13))
+        || (!ENABLE_ARCH_8 && s->thumb && (a->rd == 13 || a->rt == 13))
         || (mop == MO_64
             && (a->rt2 == 15
                 || a->rd == a->rt2
-                || (s->thumb && a->rt2 == 13)))) {
+                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
         unallocated_encoding(s);
         return true;
     }
@@ -9087,10 +9087,10 @@  static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq)
 
     /* We UNDEF for these UNPREDICTABLE cases.  */
     if (a->rn == 15 || a->rt == 15
-        || (s->thumb && a->rt == 13)
+        || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13)
         || (mop == MO_64
             && (a->rt2 == 15 || a->rt == a->rt2
-                || (s->thumb && a->rt2 == 13)))) {
+                || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) {
         unallocated_encoding(s);
         return true;
     }