| Message ID | 20190819213755.26175-20-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Convert aa32 base isa to decodetree | expand |
On Mon, 19 Aug 2019 at 22:38, Richard Henderson <richard.henderson@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate.c | 24 +++++++++++++----------- > target/arm/a32.decode | 1 + > target/arm/t32.decode | 19 +++++++++++++++++++ > 3 files changed, 33 insertions(+), 11 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index cb6296dc12..0e51289928 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -7626,6 +7626,11 @@ static void arm_skip_unless(DisasContext *s, uint32_t cond) > * Constant expanders for the decoders. > */ > > +static int negate(DisasContext *s, int x) > +{ > + return -x; > +} > + > static int times_2(DisasContext *s, int x) > { > return x * 2; > @@ -7975,6 +7980,12 @@ static bool trans_ORN_rri(DisasContext *s, arg_s_rri_rot *a) > #undef DO_ANY2 > #undef DO_CMP2 > > +static bool trans_ADR(DisasContext *s, arg_ri *a) > +{ > + store_reg_bx(s, a->rd, add_reg_for_lit(s, 15, a->imm)); > + return true; > +} > + > /* > * Multiply and multiply accumulate > */ > @@ -10670,17 +10681,8 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) > } > store_reg(s, rd, tmp); > } else { > - /* Add/sub 12-bit immediate. */ > - if (insn & (1 << 23)) { > - imm = -imm; > - } > - tmp = add_reg_for_lit(s, rn, imm); > - if (rn == 13 && rd == 13) { > - /* ADD SP, SP, imm or SUB SP, SP, imm */ > - store_sp_checked(s, tmp); > - } else { > - store_reg(s, rd, tmp); > - } > + /* Add/sub 12-bit immediate, in decodetree */ > + goto illegal_op; We seem to have lost the store_sp_checked() handling ? > } > } > } else { > diff --git a/target/arm/a32.decode b/target/arm/a32.decode > index c7f156be6d..aac991664d 100644 > --- a/target/arm/a32.decode > +++ b/target/arm/a32.decode > @@ -30,6 +30,7 @@ > &rrrr rd rn rm ra > &rrr rd rn rm > &rr rd rm > +&ri rd imm > &r rm > &i imm > &msr_reg rn r mask Should this change be in some other patch ? > diff --git a/target/arm/t32.decode b/target/arm/t32.decode > index 5116c6165a..be4e5f087c 100644 > --- a/target/arm/t32.decode > +++ b/target/arm/t32.decode > @@ -27,6 +27,7 @@ > &rrrr !extern rd rn rm ra > &rrr !extern rd rn rm > &rr !extern rd rm > +&ri !extern rd imm > &r !extern rm > &i !extern imm > &msr_reg !extern rn r mask > @@ -121,6 +122,24 @@ SBC_rri 1111 0.0 1011 . .... 0 ... .... ........ @s_rri_rot > } > RSB_rri 1111 0.0 1110 . .... 0 ... .... ........ @s_rri_rot > > +# Data processing (plain binary immediate) > + > +%imm12_26_12_0 26:1 12:3 0:8 > +%neg12_26_12_0 26:1 12:3 0:8 !function=negate > +@s0_rri_12 .... ... .... . rn:4 . ... rd:4 ........ \ > + &s_rri_rot imm=%imm12_26_12_0 rot=0 s=0 > + > +{ > + ADR 1111 0.1 0000 0 1111 0 ... rd:4 ........ \ > + &ri imm=%imm12_26_12_0 > + ADD_rri 1111 0.1 0000 0 .... 0 ... .... ........ @s0_rri_12 > +} > +{ > + ADR 1111 0.1 0101 0 1111 0 ... rd:4 ........ \ > + &ri imm=%neg12_26_12_0 > + SUB_rri 1111 0.1 0101 0 .... 0 ... .... ........ @s0_rri_12 > +} > + > # Multiply and multiply accumulate > > @s0_rnadm .... .... .... rn:4 ra:4 rd:4 .... rm:4 &s_rrrr s=0 > -- > 2.17.1 thanks -- PMM
On 8/23/19 6:04 AM, Peter Maydell wrote: >> +static bool trans_ADR(DisasContext *s, arg_ri *a) >> +{ >> + store_reg_bx(s, a->rd, add_reg_for_lit(s, 15, a->imm)); >> + return true; >> +} ... >> - if (rn == 13 && rd == 13) { >> - /* ADD SP, SP, imm or SUB SP, SP, imm */ >> - store_sp_checked(s, tmp); >> - } else { >> - store_reg(s, rd, tmp); >> - } >> + /* Add/sub 12-bit immediate, in decodetree */ >> + goto illegal_op; > > We seem to have lost the store_sp_checked() handling ? Not from ADR, which of course is ADD RD, PC, IMM, and so wouldn't match. >> +&ri rd imm >> &r rm >> &i imm >> &msr_reg rn r mask > > Should this change be in some other patch ? No, it's used by ADR. >> + ADR 1111 0.1 0000 0 1111 0 ... rd:4 ........ \ >> + &ri imm=%imm12_26_12_0 ... here. >> + ADD_rri 1111 0.1 0000 0 .... 0 ... .... ........ @s0_rri_12 The rest of the store_sp_checked handling is here in the existing ADD path. Recall STREG_SP_CHECK from patch 2. r~
On Fri, 23 Aug 2019 at 15:45, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/23/19 6:04 AM, Peter Maydell wrote: > >> +&ri rd imm > >> &r rm > >> &i imm > >> &msr_reg rn r mask > > > > Should this change be in some other patch ? > > No, it's used by ADR. > > >> + ADR 1111 0.1 0000 0 1111 0 ... rd:4 ........ \ > >> + &ri imm=%imm12_26_12_0 > > ... here. This is in t32.decode, which has its own definition of &ri. The one I was asking about was the one in a32.decode -- the addition of that line is the only change to a32.decode in this patch. thanks -- PMM
On 8/23/19 7:47 AM, Peter Maydell wrote: > On Fri, 23 Aug 2019 at 15:45, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> On 8/23/19 6:04 AM, Peter Maydell wrote: >>>> +&ri rd imm >>>> &r rm >>>> &i imm >>>> &msr_reg rn r mask >>> >>> Should this change be in some other patch ? >> >> No, it's used by ADR. >> >>>> + ADR 1111 0.1 0000 0 1111 0 ... rd:4 ........ \ >>>> + &ri imm=%imm12_26_12_0 >> >> ... here. > > This is in t32.decode, which has its own definition of &ri. > The one I was asking about was the one in a32.decode -- the > addition of that line is the only change to a32.decode in this patch. a32.decode is where all of the shared argument sets are declared; t32.decode gets the !extern markup. If I only put it in t32.decode now, I'd only have to move it later. It will eventually be used in a32.decode by MOVW/MOVT. r~
diff --git a/target/arm/translate.c b/target/arm/translate.c index cb6296dc12..0e51289928 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -7626,6 +7626,11 @@ static void arm_skip_unless(DisasContext *s, uint32_t cond) * Constant expanders for the decoders. */ +static int negate(DisasContext *s, int x) +{ + return -x; +} + static int times_2(DisasContext *s, int x) { return x * 2; @@ -7975,6 +7980,12 @@ static bool trans_ORN_rri(DisasContext *s, arg_s_rri_rot *a) #undef DO_ANY2 #undef DO_CMP2 +static bool trans_ADR(DisasContext *s, arg_ri *a) +{ + store_reg_bx(s, a->rd, add_reg_for_lit(s, 15, a->imm)); + return true; +} + /* * Multiply and multiply accumulate */ @@ -10670,17 +10681,8 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } store_reg(s, rd, tmp); } else { - /* Add/sub 12-bit immediate. */ - if (insn & (1 << 23)) { - imm = -imm; - } - tmp = add_reg_for_lit(s, rn, imm); - if (rn == 13 && rd == 13) { - /* ADD SP, SP, imm or SUB SP, SP, imm */ - store_sp_checked(s, tmp); - } else { - store_reg(s, rd, tmp); - } + /* Add/sub 12-bit immediate, in decodetree */ + goto illegal_op; } } } else { diff --git a/target/arm/a32.decode b/target/arm/a32.decode index c7f156be6d..aac991664d 100644 --- a/target/arm/a32.decode +++ b/target/arm/a32.decode @@ -30,6 +30,7 @@ &rrrr rd rn rm ra &rrr rd rn rm &rr rd rm +&ri rd imm &r rm &i imm &msr_reg rn r mask diff --git a/target/arm/t32.decode b/target/arm/t32.decode index 5116c6165a..be4e5f087c 100644 --- a/target/arm/t32.decode +++ b/target/arm/t32.decode @@ -27,6 +27,7 @@ &rrrr !extern rd rn rm ra &rrr !extern rd rn rm &rr !extern rd rm +&ri !extern rd imm &r !extern rm &i !extern imm &msr_reg !extern rn r mask @@ -121,6 +122,24 @@ SBC_rri 1111 0.0 1011 . .... 0 ... .... ........ @s_rri_rot } RSB_rri 1111 0.0 1110 . .... 0 ... .... ........ @s_rri_rot +# Data processing (plain binary immediate) + +%imm12_26_12_0 26:1 12:3 0:8 +%neg12_26_12_0 26:1 12:3 0:8 !function=negate +@s0_rri_12 .... ... .... . rn:4 . ... rd:4 ........ \ + &s_rri_rot imm=%imm12_26_12_0 rot=0 s=0 + +{ + ADR 1111 0.1 0000 0 1111 0 ... rd:4 ........ \ + &ri imm=%imm12_26_12_0 + ADD_rri 1111 0.1 0000 0 .... 0 ... .... ........ @s0_rri_12 +} +{ + ADR 1111 0.1 0101 0 1111 0 ... rd:4 ........ \ + &ri imm=%neg12_26_12_0 + SUB_rri 1111 0.1 0101 0 .... 0 ... .... ........ @s0_rri_12 +} + # Multiply and multiply accumulate @s0_rnadm .... .... .... rn:4 ra:4 rd:4 .... rm:4 &s_rrrr s=0
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate.c | 24 +++++++++++++----------- target/arm/a32.decode | 1 + target/arm/t32.decode | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-)