On Mon, 19 Aug 2019 at 22:39, Richard Henderson <richard.hender...@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/arm/translate.c | 85 ++++++++++++++++++++---------------------- > target/arm/t16.decode | 12 ++++++ > 2 files changed, 52 insertions(+), 45 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index 414c562fb3..368f0ab147 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -7474,6 +7474,11 @@ static int negate(DisasContext *s, int x) > return -x; > } > > +static int plus_2(DisasContext *s, int x) > +{ > + return x + 2; > +} > + > static int times_2(DisasContext *s, int x) > { > return x * 2; > @@ -10152,6 +10157,9 @@ static bool trans_CPS(DisasContext *s, arg_CPS *a) > { > uint32_t mask, val; > > + if (ENABLE_ARCH_6 && arm_dc_feature(s, ARM_FEATURE_M)) { > + return false; > + }
I don't think this condition is quite right. We want to do two things: (1) this is the A/R-profile CPS, so it shouldn't be decoded for any ARM_FEATURE_M CPU (2) for A/R-profile, all the CPS instructions are v6-or-better (All M-profile CPUs are at v6-or-better, which is why the legacy decoder gets away with doing its ARCH(6) check up front rather than only in the A/R-profile arm of its if statement.) > if (IS_USER(s)) {> /* Implemented as NOP in user mode. */ > return true; > @@ -10182,6 +10190,36 @@ static bool trans_CPS(DisasContext *s, arg_CPS *a) > return true; > } > > +static bool trans_CPS_v6m(DisasContext *s, arg_CPS_v6m *a) > +{ > + TCGv_i32 tmp, addr; > + > + if (!(ENABLE_ARCH_6 && arm_dc_feature(s, ARM_FEATURE_M))) { > + return false; > + } Similarly, this one need not check ENABLE_ARCH_6. That is, this is the generic M-profile CPS, it's not specific to v6M, and FEATURE_M always implies ARCH_6 anyway. Usually we name M-profile specific functions _v7m, not _v6m, for mostly historical reasons relating to our having implemented v7m first, so maybe we should follow that here. I have made a bit of an inconsistent hash of this with the v8M support, where sometimes I use _v8m because the function is only in v8M and not v7M, and sometimes _v7m because it's an M-profile function even if it happens that it only kicks in or is called for v8M CPUs. But we do not curretly have any functions with a _v6m suffix so we should probably go with _v7m here. > + if (IS_USER(s)) { > + /* Implemented as NOP in user mode. */ > + return true; > + } > + > + tmp = tcg_const_i32(a->im); > + /* FAULTMASK */ > + if (a->F) { > + addr = tcg_const_i32(19); > + gen_helper_v7m_msr(cpu_env, addr, tmp); > + tcg_temp_free_i32(addr); > + } > + /* PRIMASK */ > + if (a->I) { > + addr = tcg_const_i32(16); > + gen_helper_v7m_msr(cpu_env, addr, tmp); > + tcg_temp_free_i32(addr); > + } > + tcg_temp_free_i32(tmp); > + gen_lookup_tb(s); > + return true; > +} thanks -- PMM