RFR: 8287325: AArch64: fix virtual threads with -XX:UseBranchProtection=pac-ret [v8]
Andrew Haley
aph at openjdk.org
Thu Sep 14 08:55:45 UTC 2023
On Thu, 14 Sep 2023 05:36:43 GMT, Hao Sun <haosun at openjdk.org> wrote:
>> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 678:
>>
>>> 676:
>>> 677: intptr_t* chunk_top = chunk->start_address() + chunk_new_sp;
>>> 678: assert(_empty || ContinuationHelper::return_address_at(_orig_chunk_sp - frame::sender_sp_ret_address_offset()) == chunk->pc(), "");
>>
>> This line is way too long too.
>> Suggestion:
>>
>> if (! _empty) {
>> address *retaddr_slot = _orig_chunk_sp - frame::sender_sp_ret_address_offset();
>> assert(ContinuationHelper::return_address_at(retaddr_slot) == chunk->pc(),
>> "Saved return address is bad");
>> }
>
> Thanks for your suggestion. Updated in the latest commit.
>
> Note that `#ifdef ASSERT` is added in my commit, since variables `_empty` and `_orig_chunk_sp` belong to that scope.
Please find some other extremely long lines in this patch. One is 138 characters long! Anything much more than 80 is a candidate for attention. It's very hard to read. And please don't use empty comment strings in assertions.
All of this is about code readability.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1325610819
More information about the hotspot-dev
mailing list