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

[Nelson Chu]

On Thu, Feb 18, 2021 at 8:15 AM Jim Wilson <jimw@sifive.com> wrote:
>
> 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.

Make sense, thanks for pointing it out.

>> 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.

File a bug report is great, I think this definitely needs to be
clarified, and then be fixed.  Anyway, thanks for the approval, and
mentioning the compressed shifts issue.

Nelson