On Fri, 28 Nov 2025 12:19:03 GMT, Marc Chevalier <[email protected]> wrote:
>> 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.
Regarding Manuel's proposal: I don't think omitting brackets is a good idea.
Alternative:
} else
#endif
{
stp ...
}
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1764#discussion_r2571507484
