[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 14/64] target-arm: Use new deposit and extrac
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v4 14/64] target-arm: Use new deposit and extract ops |
Date: |
Thu, 01 Dec 2016 17:19:36 +0000 |
User-agent: |
mu4e 0.9.17; emacs 25.1.50.21 |
Richard Henderson <address@hidden> writes:
> Use the new primitives for UBFX and SBFX.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> target-arm/translate-a64.c | 79
> +++++++++++++++-------------------------------
> target-arm/translate.c | 37 +++++-----------------
> 2 files changed, 34 insertions(+), 82 deletions(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index de48747..e90487b 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -3219,67 +3219,40 @@ static void disas_bitfield(DisasContext *s, uint32_t
> insn)
> low 32-bits anyway. */
> tcg_tmp = read_cpu_reg(s, rn, 1);
>
> - /* Recognize the common aliases. */
> - if (opc == 0) { /* SBFM */
> - if (ri == 0) {
> - if (si == 7) { /* SXTB */
> - tcg_gen_ext8s_i64(tcg_rd, tcg_tmp);
> - goto done;
> - } else if (si == 15) { /* SXTH */
> - tcg_gen_ext16s_i64(tcg_rd, tcg_tmp);
> - goto done;
> - } else if (si == 31) { /* SXTW */
> - tcg_gen_ext32s_i64(tcg_rd, tcg_tmp);
> - goto done;
> - }
> - }
> - if (si == 63 || (si == 31 && ri <= si)) { /* ASR */
> - if (si == 31) {
> - tcg_gen_ext32s_i64(tcg_tmp, tcg_tmp);
> - }
> - tcg_gen_sari_i64(tcg_rd, tcg_tmp, ri);
> + /* Recognize simple(r) extractions. */
> + if (ri <= si) {
> + int len = (si - ri) + 1;
This is confusing as you have now aliased with len above.
> + if (opc == 0) { /* SBFM: ASR, SBFX, SXTB, SXTH, SXTW */
> + tcg_gen_sextract_i64(tcg_rd, tcg_tmp, ri, len);
> goto done;
> - }
> - } else if (opc == 2) { /* UBFM */
> - if (ri == 0) { /* UXTB, UXTH, plus non-canonical AND */
> - tcg_gen_andi_i64(tcg_rd, tcg_tmp, bitmask64(si + 1));
> - return;
> - }
> - if (si == 63 || (si == 31 && ri <= si)) { /* LSR */
> - if (si == 31) {
> - tcg_gen_ext32u_i64(tcg_tmp, tcg_tmp);
> - }
> - tcg_gen_shri_i64(tcg_rd, tcg_tmp, ri);
> + } else if (opc == 2) { /* UBFM: UBFX, LSR, UXTB, UXTH */
> + tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, len);
> return;
> }
> - if (si + 1 == ri && si != bitsize - 1) { /* LSL */
> - int shift = bitsize - 1 - si;
> - tcg_gen_shli_i64(tcg_rd, tcg_tmp, shift);
> - goto done;
> - }
> }
>
> - if (opc != 1) { /* SBFM or UBFM */
> - tcg_gen_movi_i64(tcg_rd, 0);
> - }
> + /* Do the bit move operation. Note that above we handled ri <= si,
> + Wd<s-r:0> = Wn<s:r>, via tcg_gen_*extract_i64. Now we handle
> + the ri > si case, Wd<32+s-r,32-r> = Wn<s:0>, via deposit. */
> + pos = (bitsize - ri) & (bitsize - 1);
> + len = si + 1;
The comment implies this is for the ri > si case but you'll still catch
ri <= si for opc = 1, e.g.:
0x331168a7 bfxil w7, w5, #17, #10
>
> - /* do the bit move operation */
> - if (si >= ri) {
In fact we seem to have subtly reversed the test here but ri <= si is
not exactly equivalent to si >= ri.
My version is as follows:
/* Recognize simple(r) extractions. */
if (si >= ri) {
/* Wd<s-r:0> = Wn<s:r> */
len = (si - ri) + 1;
if (opc == 0) { /* SBFM: ASR, SBFX, SXTB, SXTH, SXTW */
tcg_gen_sextract_i64(tcg_rd, tcg_tmp, ri, len);
goto done;
} else if (opc == 2) { /* UBFM: UBFX, LSR, UXTB, UXTH */
tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, len);
return;
}
/* opc == 1, BXFIL fall through to deposit */
tcg_gen_extract_i64(tcg_tmp, tcg_tmp, ri, len);
pos = 0;
} else {
/* Handle the ri > si case with a deposit
* Wd<32+s-r,32-r> = Wn<s:0>
*/
len = si + 1;
pos = (bitsize - ri) & (bitsize - 1);
}
I've tested that with risu and all the bitfield tests seem ok. The full
patch on top of your commit was:
target-arm: fix bxfil case
1 file changed, 13 insertions(+), 9 deletions(-)
target-arm/translate-a64.c | 22 +++++++++++++---------
modified target-arm/translate-a64.c
@@ -3220,8 +3220,9 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
tcg_tmp = read_cpu_reg(s, rn, 1);
/* Recognize simple(r) extractions. */
- if (ri <= si) {
- int len = (si - ri) + 1;
+ if (si >= ri) {
+ /* Wd<s-r:0> = Wn<s:r> */
+ len = (si - ri) + 1;
if (opc == 0) { /* SBFM: ASR, SBFX, SXTB, SXTH, SXTW */
tcg_gen_sextract_i64(tcg_rd, tcg_tmp, ri, len);
goto done;
@@ -3229,14 +3230,17 @@ static void disas_bitfield(DisasContext *s, uint32_t
insn)
tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, len);
return;
}
+ /* opc == 1, BXFIL fall through to deposit */
+ tcg_gen_extract_i64(tcg_tmp, tcg_tmp, ri, len);
+ pos = 0;
+ } else {
+ /* Handle the ri > si case with a deposit
+ * Wd<32+s-r,32-r> = Wn<s:0>
+ */
+ len = si + 1;
+ pos = (bitsize - ri) & (bitsize - 1);
}
- /* Do the bit move operation. Note that above we handled ri <= si,
- Wd<s-r:0> = Wn<s:r>, via tcg_gen_*extract_i64. Now we handle
- the ri > si case, Wd<32+s-r,32-r> = Wn<s:0>, via deposit. */
- pos = (bitsize - ri) & (bitsize - 1);
- len = si + 1;
-
if (opc == 0 && len < ri) {
/* SBFM: sign extend the destination field from len to fill
the balance of the word. Let the deposit below insert all
@@ -3245,7 +3249,7 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
len = ri;
}
- if (opc == 1) { /* BFM */
+ if (opc == 1) { /* BFM, BXFIL */
tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);
} else {
/* SBFM or UBFM: We start with zero, and we haven't modified
--
Alex Bennée
- Re: [Qemu-devel] [PATCH v4 14/64] target-arm: Use new deposit and extract ops,
Alex Bennée <=