RFR: 8305008: RISC-V: Factor out immediate checking functions from assembler_riscv.inline.hpp

Dingli Zhang dzhang at openjdk.org
Tue Mar 28 09:45:35 UTC 2023


On Tue, 28 Mar 2023 07:12:19 GMT, Feilong Jiang <fjiang at openjdk.org> wrote:

>> The call-point to 'is_imm_in_range', 'is_unsigned_imm_in_range' and 'is_offset_in_range' in assembler_riscv.inline.hpp could to be replaced with shared code function 'is_simm' and 'is_uimm'.
>> 
>> For 'is_imm_in_range', if align_bits = 0, then the call point can be changed to call 'is_simm'. If align_bits = 1, then an additional summation judgement is added to determine if it is a multiple of 2. For 'is_unsigned_imm_in_range', we can first determine if it is greater than 0, and then call 'is_uimm' further. 'is_offset_in_range' can be changed directly to call 'is_simm'. This patch also refactored for is_valid_riscv64_address and movptr.
>> 
>> This patch removed the original functions from assembler_riscv.inline.hpp and defined some fixed-width checking functions based on the number of immediate or offset bits in the spec[1]. At the same time, we also adjusted two includes to fix the compilation error after the modification.
>> 
>> This patch also renames the function 'operand_valid_for_add_immediate' to 'operand_valid_for_add_sub_immediate' to make it consistent with its usage.
>> 
>> Finally, this patch modified the return value in os::non_memory_address_word to avoid an imm64 value of -1 in MacroAssembler::movptr.
>> 
>> [1] https://github.com/riscv/riscv-isa-manual/releases/download/draft-20230131-c0b298a/riscv-spec.pdf
>> 
>> ### Testing:
>> 
>> On Unmatched with release build:
>> - [x] Tier1 tests
>> - [ ] Tier2 tests
>> - [ ] Tier3 tests
>
> src/hotspot/cpu/riscv/assembler_riscv.hpp line 2159:
> 
>> 2157: #define INSN(NAME, funct3, op)                                                               \
>> 2158:   void NAME(int32_t offset) {                                                                \
>> 2159:     assert(is_simm12(offset) && ((offset % 2) == 0), "just checking");                       \
> 
> Could you please improve the assertion error message? like `invalid imm` or something more specific.

Thanks! I will use `invalid encoding` instead.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13202#discussion_r1150317650


More information about the hotspot-dev mailing list