[PATCH] RISC-V: PR27158, fixed UJ/SB types and added CSS/CL/CS types for .insn.

[Jim Wilson]

On Wed, Jan 27, 2021 at 1:32 AM Nelson Chu <nelson.chu@sifive.com> wrote:

> Ci: Unused and removed.
>

I was curious, so I investigated.  It turns out that an early v1.7 version
of the C spec had a compressed andi with a 3-bit signed immediate.  When
that was dropped, the Ci stuff was accidentally left behind.  So this
should be dropped.

> C<: Unused and removed.
>

Similarly, there was a compressed slli.w in the C v1.7 spec that needed a 0
to 31 shift count.  This was later removed and the C< was accidentally left
behind.

However, this one got me thinking.  We still have rv32 shifts that should
take a 0 to 31 shift count, so why don't they use C<?  The issue here is
that sometime long before the port was upstreamed, the '>' support was
changed to be 0 to xlen-1 instead of 0 to 63, making it correct for both
rv32 and rv64.  Or maybe it was always written this way which would explain
why I can't find the change.  But the C> support has apparently always used
64 and has never had the change to use xlen-1.  So compressed shifts should
be using C>, but C> is possibly buggy.  This is trivial to demonstrate.

rohan:2255$ cat tmp.s
slli a0,a0,63
rohan:2256$ riscv32-unknown-elf-as -march=rv64g tmp.s
rohan:2257$ riscv32-unknown-elf-as -march=rv64gc tmp.s
rohan:2258$ riscv32-unknown-elf-as -march=rv32g tmp.s
tmp.s: Assembler messages:
tmp.s:1: Error: Improper shift amount (63)
rohan:2259$ riscv32-unknown-elf-as -march=rv32gc tmp.s
rohan:2260$ riscv32-unknown-elf-objdump -dr -Mno-aliases a.out

a.out:     file format elf32-littleriscv


Disassembly of section .text:

00000000 <.text>:
   0: 157e                 c.slli a0,0x3f
rohan:2261$

The rv32gc case looks wrong, as c.slli with a 0x3f shift count is not valid
for rv32.   Technically the ISA spec says it is reserved for non-standard
extensions (NSE), but it isn't clear if we should be allowing this.  HINTs
are nops and we allow them.  But this isn't a nop.  It looks like a bug and
that the assembler should be reporting this as an error, If someone wants
to use these encodings for a non-standard extension, I think they should be
using .insn or a local gas patch instead of using c.slli 0x3f.

Anyways, removing C< is fine, and the whole patch looks OK to me, but we
have a separate issue with rv32 compressed shifts accepting shift counts
greater than 32.  Maybe I should file a bug report for that?  We might want
to ask the ISA committee about whether we should be accepting c.slli with
0x3f shift count.

Jim