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

[Andrew Haley]

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

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

> 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

I thought about that, but it ends up being rather complex. A function that does two different things depending on a boolean parameter is something of a code smell. I'll have another look.

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

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