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