RFR: 8332265: RISC-V: Materialize pointers faster by using a temp register [v6]

Robbin Ehn rehn at openjdk.org
Fri May 24 12:20:06 UTC 2024


On Fri, 24 May 2024 10:23:49 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Fixed more comments
>>  - Fixed comments
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1703:
> 
>> 1701:   assert_different_registers(Rd, tmp, noreg);
>> 1702: 
>> 1703:   uint32_t upper18 = (addr >> 30ull);
> 
> literal suffix `ull` could be removed?
> And `uint32_t upper18 = (addr >> 30ull);` + `lui(tmp, upper18 << 12);` could be replaced with `lui(tmp, addr >> 18);`?

**ull** is there to say should be a 64-bit logical shift, appereant not doing it's jobs.
The reason it's done in two steps is first we get the values, then we adjust them for our implementation of the instructions.

Our lui() shifts down the immediate value so we must shift it up before.
While when we patch lui we use the value without shifting:

  unsigned int upper18 = (addr >> 30ull);
  Assembler::patch(instruction_address + (NativeInstruction::instruction_size * 0), 31, 12, (upper18 & 0xfffff)); 


So it can be replaced but then we are missing the calculation why it's correct,

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1706:
> 
>> 1704:   int32_t  lower30 = (addr & 0x3fffffffu);
>> 1705:   int32_t  low12   = (lower30 << 20) >> 20;
>> 1706:   int32_t  mid18   = ((lower30 - low12) >> 12);
> 
> Similar here. `mid18 = ((lower30 - low12) >> 12);` and `lui(Rd, mid18 << 12);` could be replaced with `lui(Rd, lower30 - low12);`?

Same reason as above.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19246#discussion_r1613388456
PR Review Comment: https://git.openjdk.org/jdk/pull/19246#discussion_r1613389486


More information about the hotspot-dev mailing list