RFR: 8289743: AArch64: Clean up patching logic [v2]

Andrew Dinn adinn at openjdk.org
Tue Jul 12 10:14:45 UTC 2022


On Mon, 11 Jul 2022 16:33:50 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> The current logic for patching is a mess of if-then-elses. By rearranging the logic and using a switch we can make it both easier to understand and faster.
>
> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8289743: AArch64: Clean up patching logic

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 84:

> 82: // Return the total length (in bytes) of the instructions.
> 83: int MacroAssembler::pd_patch_instruction_size(address insn_addr, address target) {
> 84:   int instructions = 1;

I don't like the fact that you have to replicate this switch and associated validation logic in the two separate methods `pd_patch_instruction_size` and `target_addr_for_insn`. How about making both of them call a common auxiliary method which will either retrieve a target address or patch it? i.e. have

`int MacroAssembler::pd_patch_instruction_size(address insn_addr, address target)`
and
`address MacroAssembler::target_addr_for_insn(address insn_addr, uint32_t insn)`

both call method

`int MacroAssembler::fetch_or_patch_target_addr(address insn_addr, uint32_t insn, address &target, boolean do_patch)`

That means you can have one copy of the dispatch logic with common verification in each end case and variant action in each case depending on the value of `do_patch`.

It also means the explanatory comment above only nees to exist in one place at the head of method `fetch_or_patch_target_addr`

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

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


More information about the hotspot-dev mailing list