RISC-V: fold redundant code in riscv_ip()
Jiawei agrees with Jan Beulich's suggestion to rename O4 in a preparatory patch before cleaning up riscv_ip().
Jiawei agrees with Jan Beulich’s suggestion to address the misleading name ‘O4’ in the RISC-V instruction parsing code (riscv_ip()) with a preparatory patch before proceeding with the main cleanup patch. This approach aims to make the cleanup process cleaner by avoiding a temporary special case related to the ‘O4’ misnomer.
- proposer
Agrees with Jan Beulich's suggestion to rename O4 in a preparatory patch before the cleanup.
“That sounds good to me. Doing the O4 rename in a preparatory patch would make this cleanup cleaner and avoid the temporary special case.”
- reviewer
Suggests inserting a patch ahead of the current one to rename O4 to avoid introducing and then deleting awkward code.
“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).”
Technical Tradeoffs
- Improved code readability and maintainability.
- Potential for subtle changes in behavior due to refactoring, requiring careful testing.
In Details
This patch series refactors the RISC-V instruction pattern matching code in binutils. riscv_ip() handles instruction parsing for the assembler and disassembler. The existing code has some redundancy in how it handles different operand types, and this patch folds the common parts. 'O4' appears to be an internal enum value with a confusing name.
For Context
This discussion concerns the RISC-V architecture support in binutils, a suite of tools for working with binary files. The specific area of focus is the instruction parsing logic, which decodes machine instructions into a human-readable format. The goal is to simplify and clarify the code related to handling instruction operands, which are the inputs and outputs of an instruction. Refactoring this code improves maintainability and reduces the risk of errors when adding support for new RISC-V extensions.