RFR: 8325821: [REDO] use "dmb.ishst+dmb.ishld" for release barrier

kuaiwei duke at openjdk.org
Tue May 21 03:04:06 UTC 2024


On Mon, 20 May 2024 13:59:31 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> he origin patch for https://bugs.openjdk.org/browse/JDK-8324186 has 2 issues:
>> 1 It show regression in some platform, like Apple silicon in mac os
>> 2 Can not handle instruction sequence like "dmb.ishld; dmb.ishst; dmb.ishld; dmb.ishld"
>> 
>> It can be fixed by:
>> 1 Enable AlwaysMergeDMB by default, only disable it in architecture we can see performance improvement (N1 or N2)
>> 2 Check the special pattern and merge the subsequent dmb.
>> 
>> It also fix a bug when code buffer is expanding, st/ld/dmb can not be merged. I added unit tests for these.
>> 
>> This patch still has a unhandled case. Insts like "dmb.ishld; dmb.ishst; dmb.ish", it will merge the last 2 instructions and can not merge all three. Because when emitting dmb.ish, if merge all previous dmbs, the code buffer will shrink the size. I think it may break some resumption and think it's not a common pattern.
>> 
>> In previous PR https://github.com/openjdk/jdk/pull/18467 , I tried an implementation to use state machine for merging. But it looks risky to pending instruction during emitting.
>
> src/hotspot/cpu/aarch64/aarch64.ad line 7841:
> 
>> 7839:   ins_encode %{
>> 7840:     __ block_comment("membar_release");
>> 7841:     __ membar(Assembler::StoreStore);
> 
> Do we need to respect `AlwaysMergeDMB`here?

Yes, usually they can be merged in macroAssembler. but  it can help to reduce the possibility of unmerged case. Thanks to point it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19278#discussion_r1607532915


More information about the hotspot-dev mailing list