On Fri, 28 Nov 2025 11:59:34 GMT, Manuel Hässig <[email protected]> wrote:
>> Marc Chevalier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5915:
>
>> 5913: #ifdef ASSERT
>> 5914: }
>> 5915: #endif
>
> Not sure if this is a good idea, but you could omit the braces on the else to
> make this a bit less busy.
I agree it's busy, but I like explicit braces, rust-style. Especially with
`#ifdef` in the middle, it looks easy to do something wrong with un-braced
blocks. I'm not so shocked by the current style but another option I see would
be
if (DEBUG_ONLY(save_fake_rfp_lr) NOT_DEBUG(false)) {
mov_immediate64(rscratch1, ((uint64_t)badRegWordVal) << 32 |
(uint64_t)badRegWordVal);
stp(rscratch1, rscratch1, Address(sp, framesize - 2 * wordSize));
} else {
stp(rfp, lr, Address(sp, framesize - 2 * wordSize));
}
I am very confident gcc will remove the branch entirely in non-debug.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1764#discussion_r2571484688
