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

[Peter Maydell]

On Mon, 18 Nov 2019 at 20:02, Richard Henderson
<address@hidden> wrote:
>
> On 11/18/19 6:53 PM, Peter Maydell wrote:
> > On Mon, 18 Nov 2019 at 13:16, Richard Henderson
> > <address@hidden> 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