From: Richard Henderson <richard.henderson@linaro.org>
To: frank.chang@sifive.com, qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
Kito Cheng <kito.cheng@sifive.com>,
Alistair Francis <Alistair.Francis@wdc.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Sagar Karandikar <sagark@eecs.berkeley.edu>
Subject: Re: [RFC 11/15] target/riscv: rvb: generalized reverse
Date: Thu, 19 Nov 2020 13:24:22 -0800 [thread overview]
Message-ID: <92347abb-7acf-b24e-6072-ff59d97bb6ba@linaro.org> (raw)
In-Reply-To: <20201118083044.13992-12-frank.chang@sifive.com>
On 11/18/20 12:29 AM, frank.chang@sifive.com wrote:
> +static target_ulong do_grev(target_ulong rs1,
> + target_ulong rs2,
> + const target_ulong masks[])
> +{
I think the masks should be placed here, and not passed in.
What you should pass in is "int bits".
> + target_ulong x = rs1;
> + int shift = 1;
> + int i = 0;
> +
> + while (shift < TARGET_LONG_BITS) {
> + if (rs2 & shift) {
> + x = do_swap(x, masks[i], shift);
> + }
> + shift <<= 1;
> + ++i;
Cleaner as for loop:
for (i = 0, shift = 1; shift < bits; i++, shift <<= 1)
> + static const target_ulong masks[] = {
> +#ifdef TARGET_RISCV32
> + 0x55555555, 0x33333333, 0x0f0f0f0f,
> + 0x00ff00ff, 0x0000ffff,
> +#else
> + 0x5555555555555555, 0x3333333333333333,
> + 0x0f0f0f0f0f0f0f0f, 0x00ff00ff00ff00ff,
> + 0x0000ffff0000ffff, 0x00000000ffffffff,
> +#endif
You don't need to replicate every entry.
dup_const(0x55, MO_8),
dup_const(0x33, MO_8),
dup_const(0x0f, MO_8),
dup_const(0xff, MO_16),
dup_const(0xffff, MO_32),
#ifdef TARGET_RISCV64
UINT32_MAX
#endif
> +target_ulong HELPER(grevw)(target_ulong rs1, target_ulong rs2)
> +{
> + static const target_ulong masks[] = {
> + 0x55555555, 0x33333333, 0x0f0f0f0f,
> + 0x00ff00ff, 0x0000ffff,
> + };
> +
> + return do_grev(rs1, rs2, masks);
This one is broken because do_grev iterated to TARGET_LONG_BITS == 64, and the
masks array is too small.
Fixed by passing in 32 as bits parameter to do_grev, as above.
> +static bool trans_grevi(DisasContext *ctx, arg_grevi *a)
> +{
> + REQUIRE_EXT(ctx, RVB);
> +
> + if (a->shamt >= TARGET_LONG_BITS) {
> + return false;
> + }
> +
> + return gen_grevi(ctx, a);
> +}
While this is ok for an initial implementation, it is worth noticing the shamt
for rev8 as a special case for tcg_gen_bswap_tl.
Otherwise, this needs the same gen_shift treatment.
r~
WARNING: multiple messages have this Message-ID (diff)
From: Richard Henderson <richard.henderson@linaro.org>
To: frank.chang@sifive.com, qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>,
Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
Alistair Francis <Alistair.Francis@wdc.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Kito Cheng <kito.cheng@sifive.com>
Subject: Re: [RFC 11/15] target/riscv: rvb: generalized reverse
Date: Thu, 19 Nov 2020 13:24:22 -0800 [thread overview]
Message-ID: <92347abb-7acf-b24e-6072-ff59d97bb6ba@linaro.org> (raw)
In-Reply-To: <20201118083044.13992-12-frank.chang@sifive.com>
On 11/18/20 12:29 AM, frank.chang@sifive.com wrote:
> +static target_ulong do_grev(target_ulong rs1,
> + target_ulong rs2,
> + const target_ulong masks[])
> +{
I think the masks should be placed here, and not passed in.
What you should pass in is "int bits".
> + target_ulong x = rs1;
> + int shift = 1;
> + int i = 0;
> +
> + while (shift < TARGET_LONG_BITS) {
> + if (rs2 & shift) {
> + x = do_swap(x, masks[i], shift);
> + }
> + shift <<= 1;
> + ++i;
Cleaner as for loop:
for (i = 0, shift = 1; shift < bits; i++, shift <<= 1)
> + static const target_ulong masks[] = {
> +#ifdef TARGET_RISCV32
> + 0x55555555, 0x33333333, 0x0f0f0f0f,
> + 0x00ff00ff, 0x0000ffff,
> +#else
> + 0x5555555555555555, 0x3333333333333333,
> + 0x0f0f0f0f0f0f0f0f, 0x00ff00ff00ff00ff,
> + 0x0000ffff0000ffff, 0x00000000ffffffff,
> +#endif
You don't need to replicate every entry.
dup_const(0x55, MO_8),
dup_const(0x33, MO_8),
dup_const(0x0f, MO_8),
dup_const(0xff, MO_16),
dup_const(0xffff, MO_32),
#ifdef TARGET_RISCV64
UINT32_MAX
#endif
> +target_ulong HELPER(grevw)(target_ulong rs1, target_ulong rs2)
> +{
> + static const target_ulong masks[] = {
> + 0x55555555, 0x33333333, 0x0f0f0f0f,
> + 0x00ff00ff, 0x0000ffff,
> + };
> +
> + return do_grev(rs1, rs2, masks);
This one is broken because do_grev iterated to TARGET_LONG_BITS == 64, and the
masks array is too small.
Fixed by passing in 32 as bits parameter to do_grev, as above.
> +static bool trans_grevi(DisasContext *ctx, arg_grevi *a)
> +{
> + REQUIRE_EXT(ctx, RVB);
> +
> + if (a->shamt >= TARGET_LONG_BITS) {
> + return false;
> + }
> +
> + return gen_grevi(ctx, a);
> +}
While this is ok for an initial implementation, it is worth noticing the shamt
for rev8 as a special case for tcg_gen_bswap_tl.
Otherwise, this needs the same gen_shift treatment.
r~
next prev parent reply other threads:[~2020-11-19 21:25 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-18 8:29 [RFC 00/15] support subsets of bitmanip extension frank.chang
2020-11-18 8:29 ` [RFC 01/15] target/riscv: reformat @sh format encoding for B-extension frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:03 ` Richard Henderson
2020-11-19 19:03 ` Richard Henderson
2020-11-18 8:29 ` [RFC 02/15] target/riscv: rvb: count leading/trailing zeros frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:24 ` Richard Henderson
2020-11-19 19:24 ` Richard Henderson
2020-11-19 19:48 ` Richard Henderson
2020-11-19 19:48 ` Richard Henderson
2020-11-18 8:29 ` [RFC 03/15] target/riscv: rvb: count bits set frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:27 ` Richard Henderson
2020-11-19 19:27 ` Richard Henderson
2020-11-18 8:29 ` [RFC 04/15] target/riscv: rvb: logic-with-negate frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:28 ` Richard Henderson
2020-11-19 19:28 ` Richard Henderson
2020-11-18 8:29 ` [RFC 05/15] target/riscv: rvb: pack two words into one register frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:43 ` Richard Henderson
2020-11-19 19:43 ` Richard Henderson
2020-11-18 8:29 ` [RFC 06/15] target/riscv: rvb: min/max instructions frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:46 ` Richard Henderson
2020-11-19 19:46 ` Richard Henderson
2020-11-18 8:29 ` [RFC 07/15] target/riscv: rvb: sign-extend instructions frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 19:48 ` Richard Henderson
2020-11-19 19:48 ` Richard Henderson
2020-11-18 8:29 ` [RFC 08/15] target/riscv: rvb: single-bit instructions frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 20:05 ` Richard Henderson
2020-11-19 20:05 ` Richard Henderson
2020-11-19 20:35 ` Richard Henderson
2020-11-19 20:35 ` Richard Henderson
2020-11-19 21:04 ` Richard Henderson
2020-11-19 21:04 ` Richard Henderson
2020-12-04 17:10 ` Frank Chang
2020-12-04 17:10 ` Frank Chang
2020-11-18 8:29 ` [RFC 09/15] target/riscv: rvb: shift ones frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 20:54 ` Richard Henderson
2020-11-19 20:54 ` Richard Henderson
2020-11-18 8:29 ` [RFC 10/15] target/riscv: rvb: rotate (left/right) frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 21:06 ` Richard Henderson
2020-11-19 21:06 ` Richard Henderson
2020-11-18 8:29 ` [RFC 11/15] target/riscv: rvb: generalized reverse frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 21:24 ` Richard Henderson [this message]
2020-11-19 21:24 ` Richard Henderson
2020-11-18 8:29 ` [RFC 12/15] target/riscv: rvb: generalized or-combine frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 21:28 ` Richard Henderson
2020-11-19 21:28 ` Richard Henderson
2020-11-18 8:29 ` [RFC 13/15] target/riscv: rvb: address calculation frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 21:38 ` Richard Henderson
2020-11-19 21:38 ` Richard Henderson
2020-11-18 8:29 ` [RFC 14/15] target/riscv: rvb: add/sub with postfix zero-extend frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 22:15 ` Richard Henderson
2020-11-19 22:15 ` Richard Henderson
2020-11-18 8:29 ` [RFC 15/15] target/riscv: rvb: support and turn on B-extension from command line frank.chang
2020-11-18 8:29 ` frank.chang
2020-11-19 18:54 ` Alistair Francis
2020-11-19 18:54 ` Alistair Francis
2020-11-20 3:02 ` Kito Cheng
2020-11-20 3:02 ` Kito Cheng
2020-11-20 16:24 ` Alistair Francis
2020-11-20 16:24 ` Alistair Francis
2020-11-23 1:22 ` Frank Chang
2020-11-23 1:22 ` Frank Chang
2020-11-19 22:26 ` [RFC 00/15] support subsets of bitmanip extension Richard Henderson
2020-11-20 1:45 ` Frank Chang
2020-11-20 1:45 ` Frank Chang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=92347abb-7acf-b24e-6072-ff59d97bb6ba@linaro.org \
--to=richard.henderson@linaro.org \
--cc=Alistair.Francis@wdc.com \
--cc=frank.chang@sifive.com \
--cc=kbastian@mail.uni-paderborn.de \
--cc=kito.cheng@sifive.com \
--cc=palmer@dabbelt.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=sagark@eecs.berkeley.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.