| Message ID | 20191117090621.32425-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: two fixes for ldrex/strex | expand |
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
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~
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
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~
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 --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; }
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(-)