[02/19] RISC-V: fold redundant code in riscv_ip()

Jiawei jiawei@iscas.ac.cn
Sat May 9 05:45:07 GMT 2026


在 2026/5/8 15:06, Jan Beulich 写道:
> On 28.04.2026 03:52, Jiawei wrote:
>>> The parsing of 'F' and 'O' .insn operands is pretty redundant. Have only a
>>> single instance each of common code, with the inner switch()es merely
>>> handling the actual value insertion. This in particular simplifies the
>>> addition of new sub-forms.
>>> ---
>>> Since it's entirely internal, can't we rename O4 to either O7 or
>>> (describing merely the non-fixed bits) O5? I wonder why O4 was used in the
>>> first place ...
>>>
>> The refactoring looks reasonable overall, but I have one question about the
>> O2 case.
>>
>> Before this change, O2 rejected values >= 3, so the accepted range was
>> 0...2. With the new common check based on 1U << regno, O2 now accepts
>> 0...3.
>>
>> Is this intended? If O2 is meant to describe a two-bit field, then the new
>> range looks natural, but the old code seemed to deliberately reject 3.
> I've fixed this.
>
>> The O4 part looks equivalent to me, apart from the split diagnostics. I also
>> agree that O4 is a misleading internal name; O7 would make the field width
>> clearer if we decide to rename it in a follow-up.
> For this I'm rather inclined to insert a patch ahead of this one. Then the
> odd
>
> 	      /* O4 is a misnomer, really describing a 7-bit field.  */
> 	      if (regno == 4)
> 		regno += 3;
>
> won't even need introducing (just to later delete it again).
>
> Jan
Thanks, Jan.

That sounds good to me. Doing the O4 rename in a preparatory patch would 
make
this cleanup cleaner and avoid the temporary special case.

Best regards,
Jiawei
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20260509/cff64aa1/attachment.htm>


More information about the Binutils mailing list