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

[Andrew Dinn]

On Tue, 12 Jul 2022 10:12:39 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>> 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`

n.b. I forgot to explain but I hope it is obvious that `target` is passed as a reference so it can be an input when `do_patch == true` and an output when `do_patch == false`. The return value is the instruction count in either case.

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

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