RFR: 8266609: AArch64: include FP/LR space in LIR_Assembler::initial_frame_size_in_bytes()
Nick Gasson
ngasson at openjdk.java.net
Fri May 7 01:44:57 UTC 2021
On Thu, 6 May 2021 10:28:32 GMT, Andrew Haley <aph at openjdk.org> wrote:
>> LIR_Assembler::initial_frame_size_in_bytes() returns the frame size
>> without the additional 2*wordSize needed to store FP/LR (i.e. just the
>> space required for the C1 locals). When we pass this value to
>> MacroAssembler build_frame and remove_frame we need to remember to add
>> back the 2*wordSize. This patch changes initial_frame_size_in_bytes()
>> to return the full frame size including the space for FP/LR. The PPC
>> and S390 ports already do this, and it also matches the behaviour of
>> C->output()->frame_size_in_bytes() in C2.
>>
>> This change may seem a bit trivial in mainline but the Valhalla lworld
>> branch makes more calls to build_frame/remove_frame in C1 and keeping
>> track of whether "framesize" is with or without FP/LR quickly becomes
>> confusing.
>>
>> Tested tier1 on AArch64. The only use of this function is as input to
>> C1_MacroAssembler build_frame() and remove_frame() so seems safe.
>
> src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 377:
>
>> 375: // if rounding, must let FrameMap know!
>> 376:
>> 377: return in_bytes(frame_map()->framesize_in_bytes());
>
> Umm, really? `framesize_in_bytes()` returns a value that has to be converted to bytes?
`framesize_in_bytes()` returns a struct ByteSize that needs to be converted to an int with `in_bytes()`. The extra _in_bytes seems a bit redundant but it's not my name :-)
-------------
PR: https://git.openjdk.java.net/jdk/pull/3898
More information about the hotspot-dev
mailing list