[PATCH] RISC-V: map zext.h to pack/packw if Zbkb is enabled

[Hau Hsu]

Hi Neeson, Jan,

Thanks for your review. Let me add more context for this patch.

zext.h and pack/packw share the same encoding was discussed in 
https://github.com/riscv/riscv-bitmanip/issues/170 (Jan. 2022) and
https://github.com/riscv/riscv-isa-manual/issues/1475  <https://github.com/riscv/riscv-isa-manual/issues/1475>(Jun. 2024)

In the first thread, Jim Wilson mentioned that

> Also note that GNU as supports zext.h as a macro that expands to two shifts when zbb is not supported. packw is not supported without zbkb.
> Hmm. Looks like the assembler is missing a case where we can support zext.h if zbkb but not zbb by assembling it as a packw. That looks like a bug, but an obscure one, since I expect zbb to be much > more common than zbkb.

And the second thread, Andrew Waterman said

> ... since pack[w] is a superset of zext.h, it makes sense for zext.h to use the pack[w] encoding in any situation that the full pack[w] instruction is provided (which for now I guess just means Zbkb).

Thus we have this patch.

And if this patch is feasible, I'll send v2 that follows Jan's suggestion to use alias instead of moving opcode table around.


Thanks!

Hau Hsu






> On Aug 2, 2024, at 11:43 AM, Nelson Chu <nelson@rivosinc.com> wrote:
> 
> 
> 
> On Fri, Aug 2, 2024 at 11:29 AM Hau Hsu <hau.hsu@sifive.com <mailto:hau.hsu@sifive.com>> wrote:
>> `zext.h` is zero-extend halfword instruction that belongs to Zbb.
>> Currently `zext.h` falls back to 2 shifts if Zbb is not enabled.
>> However the encoding and operation is a special case of
>> `pack/packw rd, rs1, rs2`, which belongs to Zbkb. The instructions pack
>> the low halves of rs1 and rs2 into rd.  When rs2 is zero (x0), they
>> behave like zero-extend instruction, and the encoding are exactly the
>> same as zext.h.
>> 
>> Thus we can map `zext.h` to `pack` or `packw` (rv64) if Zbkb is enabled,
>> instead of 2 shifts. This reduces one instruction.
>> 
>> This patch does this by making `zext.h` also available for Zbkb, and
>> moving `pack`, `packw` fowrard in the opcode table for disassemble.
>> 
>> opcode/
>>     * riscv-opc.c (riscv_opcodes): Move forward entries for `pack` and
>>       `packw` instructions. Update `zext.h` entries to use
>>       `ZBB_OR_ZBKB` instruction class.
>> 
>> gas/
>>     * testsuite/gas/riscv/zext-to-pack.s: Add test for mapping zext to
>>       pack/packw.
>>     * testsuite/gas/riscv/zext-to-pack.d: Likewise.
>>     * testsuite/gas/riscv/zext-to-packw.d: Likewise.
>> ---
>>  gas/testsuite/gas/riscv/zext-to-pack.d  | 11 +++++++++++
>>  gas/testsuite/gas/riscv/zext-to-pack.s  |  2 ++
>>  gas/testsuite/gas/riscv/zext-to-packw.d | 11 +++++++++++
>>  opcodes/riscv-opc.c                     |  8 ++++----
>>  4 files changed, 28 insertions(+), 4 deletions(-)
>>  create mode 100644 gas/testsuite/gas/riscv/zext-to-pack.d
>>  create mode 100644 gas/testsuite/gas/riscv/zext-to-pack.s
>>  create mode 100644 gas/testsuite/gas/riscv/zext-to-packw.d
> 
> I think you should update the spec and have a new version first since it seems zext.h only belongs to zbb rather than zbkb according to v1.0.
> https://github.com/riscv/riscv-bitmanip/blob/main/bitmanip/zbkb.adoc
> 
> Nelson

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240805/066475bf/attachment.htm>