diff mbox series

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

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

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(-)

-- 
2.17.1

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
diff mbox series

Patch

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;
     }