RFR: 8294087: RISC-V: RVC: Fix a potential alignment issue and add more alignment assertions for the patchable calls/nops [v4]

Xiaolin Zheng xlinzheng at openjdk.org
Wed Sep 21 12:28:40 UTC 2022


On Wed, 21 Sep 2022 12:00:20 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Xiaolin Zheng has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Modifications as review comments again
>>  - Revert "(Revertible) Just to see which code style is better"
>>    
>>    This reverts commit 229d535e93ceeff4f6ba32a2c882599e2999999b.
>
> src/hotspot/cpu/riscv/nativeInst_riscv.cpp line 274:
> 
>> 272: 
>> 273:   // Must be 4 byte aligned
>> 274:   MacroAssembler::assert_alignment(verified_entry);
> 
> So, if the intent to pass explicit `NativeInsn::insn_size` everywhere we say "Must be 4 bytes aligned", then we should probably pass it here too. Or, drop the explicit `NativeInsn::insn_size` everywhere else?

Well, if I understand correctly, as we want to make it clean, we won't explicitly pass `NativeInstruction::instruction_size` as an argument of `__ assert_alignment()`. And it seems currently all the call sites of `__ assert_alignment()` have already dropped the explicit argument, including this position. So... haven't we already succeeded in dropping them?

The `__ align()` API needs an explicit argument as alignment, so when calling that one, the `NativeInstruction::instruction_size` is passed explicitly.

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

PR: https://git.openjdk.org/jdk/pull/10370


More information about the hotspot-dev mailing list