| Message ID | 20200815013145.539409-5-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: SVE2 preparatory patches | expand |
On Sat, 15 Aug 2020 at 02:31, Richard Henderson <richard.henderson@linaro.org> wrote: > > We want to ensure that access is checked by the time we ask > for a specific fp/vector register. We want to ensure that > we do not emit two lots of code to raise an exception. > > But sometimes it's difficult to cleanly organize the code > such that we never pass through sve_check_access exactly once. > Allow multiple calls so long as the result is true, that is, > no exception to be raised. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate.h | 1 + > target/arm/translate-a64.c | 27 ++++++++++++++++----------- > 2 files changed, 17 insertions(+), 11 deletions(-) Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > diff --git a/target/arm/translate.h b/target/arm/translate.h > index 16f2699ad7..ad7972eb22 100644 > --- a/target/arm/translate.h > +++ b/target/arm/translate.h > @@ -64,6 +64,7 @@ typedef struct DisasContext { > * that it is set at the point where we actually touch the FP regs. > */ > bool fp_access_checked; > + bool sve_access_checked; Is there anywhere it's worthwhile to put in an equivalent of assert_fp_access_checked() for sve_access_checked, or is there no point that's both (a) common to SVE accesses and (b) not common to generic FP accesses ? One could put it in pred_full_reg_offset() I suppose but I dunno if that really gains us much. The existing fp_access_checked will catch "forgot the check entirely" which seems more likely as a bug than "put in the FP check when we wanted SVE". thanks -- PMM
On 8/24/20 9:59 AM, Peter Maydell wrote: >> + bool sve_access_checked; > > Is there anywhere it's worthwhile to put in an equivalent > of assert_fp_access_checked() for sve_access_checked, or is > there no point that's both (a) common to SVE accesses and > (b) not common to generic FP accesses ? One could put it > in pred_full_reg_offset() I suppose but I dunno if that > really gains us much. The existing fp_access_checked will > catch "forgot the check entirely" which seems more likely > as a bug than "put in the FP check when we wanted SVE". While adding one to pred_full_ref_offset() might be useful, there are plenty of sve instructions that don't touch predicate registers. I suppose I could make vec_full_reg_offset() be different between translate-a64.c and translate-sve.c, rather than sharing it via translate-a64.h. r~
diff --git a/target/arm/translate.h b/target/arm/translate.h index 16f2699ad7..ad7972eb22 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -64,6 +64,7 @@ typedef struct DisasContext { * that it is set at the point where we actually touch the FP regs. */ bool fp_access_checked; + bool sve_access_checked; /* ARMv8 single-step state (this is distinct from the QEMU gdbstub * single-step support). */ diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 534c3ff5f3..42aa695dff 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1175,18 +1175,18 @@ static void do_vec_ld(DisasContext *s, int destidx, int element, * unallocated-encoding checks (otherwise the syndrome information * for the resulting exception will be incorrect). */ -static inline bool fp_access_check(DisasContext *s) +static bool fp_access_check(DisasContext *s) { - assert(!s->fp_access_checked); - s->fp_access_checked = true; + if (s->fp_excp_el) { + assert(!s->fp_access_checked); + s->fp_access_checked = true; - if (!s->fp_excp_el) { - return true; + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); + return false; } - - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); - return false; + s->fp_access_checked = true; + return true; } /* Check that SVE access is enabled. If it is, return true. @@ -1195,10 +1195,14 @@ static inline bool fp_access_check(DisasContext *s) bool sve_access_check(DisasContext *s) { if (s->sve_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_sve_access_trap(), - s->sve_excp_el); + assert(!s->sve_access_checked); + s->sve_access_checked = true; + + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_sve_access_trap(), s->sve_excp_el); return false; } + s->sve_access_checked = true; return fp_access_check(s); } @@ -14548,6 +14552,7 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s) s->base.pc_next += 4; s->fp_access_checked = false; + s->sve_access_checked = false; if (dc_isar_feature(aa64_bti, s)) { if (s->base.num_insns == 1) {
We want to ensure that access is checked by the time we ask for a specific fp/vector register. We want to ensure that we do not emit two lots of code to raise an exception. But sometimes it's difficult to cleanly organize the code such that we never pass through sve_check_access exactly once. Allow multiple calls so long as the result is true, that is, no exception to be raised. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate.h | 1 + target/arm/translate-a64.c | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) -- 2.25.1