RISC-V: fold redundant code in riscv_ip()
Jan Beulich acknowledges and will correct an issue in the RISC-V instruction parsing code identified by Jiawei.
Jan Beulich responds to a review by Jiawei regarding a patch that folds redundant code in the riscv_ip() function. Jiawei pointed out that the patch inadvertently changed the accepted range of values for the ‘O2’ operand. Jan acknowledges the issue and indicates that they will correct it.
- reviewer
Identifies that the patch changed the accepted range of values for the O2 operand, which may be unintentional.
“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?”
- other
Acknowledges the issue and states that they will correct it.
“No, I will need to correct that. Thanks for spotting.”
Technical Tradeoffs
- The original patch simplified the code but introduced a bug.
- The corrected patch will maintain the original behavior while improving code readability.
In Details
This thread discusses a refactoring of the RISC-V instruction parsing in the GNU assembler. The specific issue is within the riscv_ip() function, which handles instruction pseudo-ops. The concern is an unintended change to the allowed range of an operand due to the refactoring.
For Context
This discussion revolves around improvements to the RISC-V assembler in the GNU binutils. The assembler translates human-readable assembly code into machine code that the processor can execute. The patch in question aimed to simplify the code that parses RISC-V instructions, but a reviewer noticed that it inadvertently changed the valid range of values for one part of an instruction, potentially leading to incorrect code generation.