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

[Jan Beulich]

On 29.07.2025 17:19, Craig Topper wrote:
> 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.

No idea how things are for llvm, but gas doesn't support RV128 yet, aiui.
Hence these insns likely should never have been there in the first place?

Jan

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