[PATCH] RISC-V: PR33216, Allow c.slli, c.srai, c.srli with 0 immediate as a hint

[Craig Topper]

I left them in as aliases when I implemented this in LLVM last week. I was
trying to not break user code that may exist. binutils and LLVM both have a
number of aliases for backward compatibility.  For example, "move",
"sbreak", "scall", "frsr", "fssr", "fmv.s.x", 'fmv.x.s". So I thought this
was standard practice.

On Tue, Jul 29, 2025 at 8:04 AM Nelson Chu <nelson@rivosinc.com> wrote:

>
>
> On Tue, Jul 29, 2025 at 3:15 PM Jan Beulich <jbeulich@suse.com> wrote:
>
>> On 29.07.2025 06:45, Nelson Chu wrote:
>> > The original patch,
>> > e6f372ba661bb0d8eec1e22a6dc1ad9937336e4d
>> >
>> > Since recently c.slli64, c.srai64, and c.srli64 have been removed from
>> the
>> > riscv-isa-manual, c.slli, c.srli, and c.srai with 0 immediate are now
>> listed
>> > as hints,
>> > https://github.com/riscv/riscv-isa-manual/pull/1942 and
>> https://github.com/riscv/riscv-isa-manual/pull/2093
>> >
>> > So allow c.slli, c.srli, and c.srai with 0 immediate as a hint.  Also
>> allow to
>> > assemble slli, srli and srai with 0 immediate to hint c.slli, c.srli
>> and c.srai
>> > when rvc is enabled.  The c.slli64, c.srai64, and c.srli64 should be
>> kept as
>> > aliases, so dis-assembler should disassemble to c.slli, c.srli, and
>> c.srai with
>> > 0 immediate.
>>
>> I don't quite understand the point of retaining ...
>>
>> > @@ -1199,12 +1174,12 @@ const struct riscv_opcode riscv_opcodes[] =
>> >  {"c.and",      0, INSN_CLASS_ZCA,   "Cs,Ct",     MATCH_C_AND,
>> MASK_C_AND, match_opcode, 0 },
>> >  {"c.or",       0, INSN_CLASS_ZCA,   "Cs,Ct",     MATCH_C_OR,
>> MASK_C_OR, match_opcode, 0 },
>> >  {"c.xor",      0, INSN_CLASS_ZCA,   "Cs,Ct",     MATCH_C_XOR,
>> MASK_C_XOR, match_opcode, 0 },
>> > -{"c.slli",     0, INSN_CLASS_ZCA,   "d,C>",      MATCH_C_SLLI,
>> MASK_C_SLLI, match_c_slli, 0 },
>> > -{"c.srli",     0, INSN_CLASS_ZCA,   "Cs,C>",     MATCH_C_SRLI,
>> MASK_C_SRLI, match_c_slli, 0 },
>> > -{"c.srai",     0, INSN_CLASS_ZCA,   "Cs,C>",     MATCH_C_SRAI,
>> MASK_C_SRAI, match_c_slli, 0 },
>> > -{"c.slli64",   0, INSN_CLASS_ZCA,   "d",         MATCH_C_SLLI64,
>> MASK_C_SLLI64, match_c_slli64, 0 },
>> > -{"c.srli64",   0, INSN_CLASS_ZCA,   "Cs",        MATCH_C_SRLI64,
>> MASK_C_SRLI64, match_c_slli64, 0 },
>> > -{"c.srai64",   0, INSN_CLASS_ZCA,   "Cs",        MATCH_C_SRAI64,
>> MASK_C_SRAI64, match_c_slli64, 0 },
>> > +{"c.slli64",   0, INSN_CLASS_ZCA,   "d",         MATCH_C_SLLI64,
>> MASK_C_SLLI64, match_c_slli64, INSN_ALIAS },
>> > +{"c.srli64",   0, INSN_CLASS_ZCA,   "Cs",        MATCH_C_SRLI64,
>> MASK_C_SRLI64, match_c_slli64, INSN_ALIAS },
>> > +{"c.srai64",   0, INSN_CLASS_ZCA,   "Cs",        MATCH_C_SRAI64,
>> MASK_C_SRAI64, match_c_slli64, INSN_ALIAS },
>>
>> ... these three. The assembler should reject them now, shouldn't it? And
>> the
>> disassembler will never use them: If use of aliases is enabled, the
>> earlier
>> entries which aren't c. prefixed would be used, whereas is aliases are
>> disabled, ...
>>
>> > +{"c.slli",     0, INSN_CLASS_ZCA,   "d,C>",      MATCH_C_SLLI,
>> MASK_C_SLLI, match_opcode, 0 },
>> > +{"c.srli",     0, INSN_CLASS_ZCA,   "Cs,C>",     MATCH_C_SRLI,
>> MASK_C_SRLI, match_opcode, 0 },
>> > +{"c.srai",     0, INSN_CLASS_ZCA,   "Cs,C>",     MATCH_C_SRAI,
>> MASK_C_SRAI, match_opcode, 0 },
>>
>> ... these three would be used.
>>
>
> Hi Jan,
>
> Yeah sounds reasonable, just remove the support of c.slli64, c.srli64,
> c.srai64 since the spec already removed them.
>
> Hi Craig,
>
> Does llvm also not support these c.s[ll|rl|ra]i64 instructions?  If llvm
> does support then probably because of the compatible issue?
>
> Thanks
> Nelson
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20250729/a90016ff/attachment.htm>