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