diff mbox series

[03/28] tcg/aarch64: Support bswap flags

Message ID 20210614083800.1166166-4-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: bswap improvements | expand

Commit Message

Richard Henderson June 14, 2021, 8:37 a.m. UTC 
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 tcg/aarch64/tcg-target.c.inc | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
2.25.1

Comments

Peter Maydell June 21, 2021, 2:01 p.m. UTC | #1 
On Mon, 14 Jun 2021 at 09:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  tcg/aarch64/tcg-target.c.inc | 12 ++++++++++++

>  1 file changed, 12 insertions(+)

>

> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc

> index 27cde314a9..f72218b036 100644

> --- a/tcg/aarch64/tcg-target.c.inc

> +++ b/tcg/aarch64/tcg-target.c.inc

> @@ -2187,12 +2187,24 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,

>          tcg_out_rev64(s, a0, a1);

>          break;

>      case INDEX_op_bswap32_i64:

> +        tcg_out_rev32(s, a0, a1);

> +        if (a2 & TCG_BSWAP_OS) {

> +            tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a0);

> +        }


Side note: it's rather confusing that tcg_out_rev32() doesn't
emit a REV32 insn (it emits REV with sf==0).

> +        break;

>      case INDEX_op_bswap32_i32:

>          tcg_out_rev32(s, a0, a1);

>          break;

>      case INDEX_op_bswap16_i64:

>      case INDEX_op_bswap16_i32:

>          tcg_out_rev16(s, a0, a1);

> +        if (a2 & TCG_BSWAP_OS) {

> +            /* Output must be sign-extended. */

> +            tcg_out_sxt(s, ext, MO_16, a0, a0);

> +        } else if ((a2 & (TCG_BSWAP_IZ | TCG_BSWAP_OZ)) == TCG_BSWAP_OZ) {

> +            /* Output must be zero-extended, but input isn't. */

> +            tcg_out_uxt(s, MO_16, a0, a0);

> +        }

>          break;


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Richard Henderson June 21, 2021, 2:04 p.m. UTC | #2 
On 6/21/21 7:01 AM, Peter Maydell wrote:
> On Mon, 14 Jun 2021 at 09:41, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>   tcg/aarch64/tcg-target.c.inc | 12 ++++++++++++

>>   1 file changed, 12 insertions(+)

>>

>> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc

>> index 27cde314a9..f72218b036 100644

>> --- a/tcg/aarch64/tcg-target.c.inc

>> +++ b/tcg/aarch64/tcg-target.c.inc

>> @@ -2187,12 +2187,24 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,

>>           tcg_out_rev64(s, a0, a1);

>>           break;

>>       case INDEX_op_bswap32_i64:

>> +        tcg_out_rev32(s, a0, a1);

>> +        if (a2 & TCG_BSWAP_OS) {

>> +            tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a0);

>> +        }

> 

> Side note: it's rather confusing that tcg_out_rev32() doesn't

> emit a REV32 insn (it emits REV with sf==0).


Mm, yes.  I'll tidy that up in a separate patch.


r~
Richard Henderson June 21, 2021, 6:12 p.m. UTC | #3 
On 6/21/21 7:01 AM, Peter Maydell wrote:
> Side note: it's rather confusing that tcg_out_rev32() doesn't

> emit a REV32 insn (it emits REV with sf==0).


Which is REV with SF=0 also has OPC=10, which is REV32.

So I think it's a simple case of the assembly mnemonics not matching up with the machine 
instructions as nicely as all that.


r~
Peter Maydell June 21, 2021, 7:40 p.m. UTC | #4 
On Mon, 21 Jun 2021 at 19:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> On 6/21/21 7:01 AM, Peter Maydell wrote:

> > Side note: it's rather confusing that tcg_out_rev32() doesn't

> > emit a REV32 insn (it emits REV with sf==0).

>

> Which is REV with SF=0 also has OPC=10, which is REV32.


No, REV32 has SF=1. The two operations are different:

 REV <Wd>, <Wn> -- swaps byte order of the bottom 32 bits
                   (zeroes the top half of Xd, as usual for Wn ops)
 REV32 <Xd>, <Xn> -- swaps byte order of bottom 32 bits and
                     also swaps byte order of top 32 bits
                     (ie it is a 64-bit to 64-bit operation
                      which does does two bswap32()s)

-- PMM
Richard Henderson June 21, 2021, 7:50 p.m. UTC | #5 
On 6/21/21 12:40 PM, Peter Maydell wrote:
> On Mon, 21 Jun 2021 at 19:12, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> On 6/21/21 7:01 AM, Peter Maydell wrote:

>>> Side note: it's rather confusing that tcg_out_rev32() doesn't

>>> emit a REV32 insn (it emits REV with sf==0).

>>

>> Which is REV with SF=0 also has OPC=10, which is REV32.

> 

> No, REV32 has SF=1. The two operations are different:

> 

>   REV <Wd>, <Wn> -- swaps byte order of the bottom 32 bits

>                     (zeroes the top half of Xd, as usual for Wn ops)

>   REV32 <Xd>, <Xn> -- swaps byte order of bottom 32 bits and

>                       also swaps byte order of top 32 bits

>                       (ie it is a 64-bit to 64-bit operation

>                        which does does two bswap32()s)


Ignore the assembler mnemonic and look at the opcode:

REV   Wd,Wn   = SF=0, OPC=10
REV32 Xd,Xn   = SF=1, OPC=10
REV   Xd,Xn   = SF=1, OPC=11

REV(Wd,Wd) = (uint32_t)REV32(Xd,Xd)
i.e. the usual interpretation of sf=0.


r~
Peter Maydell June 21, 2021, 7:52 p.m. UTC | #6 
On Mon, 21 Jun 2021 at 20:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> On 6/21/21 12:40 PM, Peter Maydell wrote:

> > On Mon, 21 Jun 2021 at 19:12, Richard Henderson

> > <richard.henderson@linaro.org> wrote:

> >>

> >> On 6/21/21 7:01 AM, Peter Maydell wrote:

> >>> Side note: it's rather confusing that tcg_out_rev32() doesn't

> >>> emit a REV32 insn (it emits REV with sf==0).

> >>

> >> Which is REV with SF=0 also has OPC=10, which is REV32.

> >

> > No, REV32 has SF=1. The two operations are different:

> >

> >   REV <Wd>, <Wn> -- swaps byte order of the bottom 32 bits

> >                     (zeroes the top half of Xd, as usual for Wn ops)

> >   REV32 <Xd>, <Xn> -- swaps byte order of bottom 32 bits and

> >                       also swaps byte order of top 32 bits

> >                       (ie it is a 64-bit to 64-bit operation

> >                        which does does two bswap32()s)

>

> Ignore the assembler mnemonic and look at the opcode:


...but the point is that tcg_out_rev32() is not doing the
thing that the assembler insn REV32 does, so ignoring the
mnemonic is missing the point.

> REV   Wd,Wn   = SF=0, OPC=10

> REV32 Xd,Xn   = SF=1, OPC=10

> REV   Xd,Xn   = SF=1, OPC=11

>

> REV(Wd,Wd) = (uint32_t)REV32(Xd,Xd)

> i.e. the usual interpretation of sf=0.


You could look at it that way, but that's not the way
the insn mnemonics have been defined...

-- PMM
diff mbox series

Patch

diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index 27cde314a9..f72218b036 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -2187,12 +2187,24 @@  static void tcg_out_op(TCGContext *s, TCGOpcode opc,
         tcg_out_rev64(s, a0, a1);
         break;
     case INDEX_op_bswap32_i64:
+        tcg_out_rev32(s, a0, a1);
+        if (a2 & TCG_BSWAP_OS) {
+            tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a0);
+        }
+        break;
     case INDEX_op_bswap32_i32:
         tcg_out_rev32(s, a0, a1);
         break;
     case INDEX_op_bswap16_i64:
     case INDEX_op_bswap16_i32:
         tcg_out_rev16(s, a0, a1);
+        if (a2 & TCG_BSWAP_OS) {
+            /* Output must be sign-extended. */
+            tcg_out_sxt(s, ext, MO_16, a0, a0);
+        } else if ((a2 & (TCG_BSWAP_IZ | TCG_BSWAP_OZ)) == TCG_BSWAP_OZ) {
+            /* Output must be zero-extended, but input isn't. */
+            tcg_out_uxt(s, MO_16, a0, a0);
+        }
         break;
 
     case INDEX_op_ext8s_i64: