[jdk17u-dev] RFR: 8265263: AArch64: Combine vneg with right shift count [v3]
Paul Hohensee
phh at openjdk.org
Tue Oct 25 22:34:44 UTC 2022
On Thu, 20 Oct 2022 16:13:40 GMT, Dmitry Chuyko <dchuyko at openjdk.org> wrote:
>> This is a performance improvement for AArch64. There are several differences from the original change.
>>
>> https://bugs.openjdk.org/browse/JDK-8267356 (Vector API SVE codegen support) is not in 17u, so `UseSVE == 0` parts in predicates are missing/excluded.
>>
>> https://bugs.openjdk.org/browse/JDK-8288445 (C2 compilation fails) is a subsequent bugfix already backported in 17u, so some `immI` arguments in rules became `immI_positive`.
>>
>> https://bugs.openjdk.org/browse/JDK-8277239 (SIGSEGV in vrshift_reg_maskedNode::emit) is also related to Vector API and is not in 17u, so `!n->as_ShiftV()->is_var_shift()` is replaced by `VectorNode::is_vshift_cnt(n->in(2))`. This substitution may raise doubts.
>>
>> Testing: jtreg test/hotspot/jtreg/compiler, tier1, tier2 on aarch64.
>>
>> Performance improvements in the added benchmark VectorShiftRight on Graviton 2 for default size=1024 correspond to the original review:
>>
>>
>> rShiftByte 16%
>> rShiftInt 27%
>> rShiftLong 16%
>> rShiftShort 20%
>> urShiftByte 0%
>> urShiftChar 20%
>> urShiftInt 27%
>> urShiftLong 16%
>
> Dmitry Chuyko has updated the pull request incrementally with one additional commit since the last revision:
>
> Fixed m4 mismatch
I'm not an expert on this code, but I don't think VectorNode::is_vshift_cnt(n->in(2)) is equivalent to !n->as_ShiftV()->is_var_shift(). is_vshift_cnt() just checks for LShiftCntV and RShiftCntV
bool VectorNode::is_vshift_cnt(Node* n) {
switch (n->Opcode()) {
case Op_LShiftCntV:
case Op_RShiftCntV:
return true;
default:
return false;
}
}
but these operators can have variable shift counts, see in aarch64_sve.ad
instruct vshiftcntB(vReg dst, iRegIorL2I cnt) %{
predicate(UseSVE > 0 && n->as_Vector()->length() >= 16 &&
(n->bottom_type()->is_vect()->element_basic_type() == T_BYTE));
match(Set dst (LShiftCntV cnt));
match(Set dst (RShiftCntV cnt));
format %{ "sve_dup $dst, $cnt\t# vector shift count (sve) (B)" %}
ins_encode %{
__ sve_dup(as_FloatRegister($dst$$reg), __ B, as_Register($cnt$$reg));
%}
ins_pipe(pipe_slow);
%}
-------------
Changes requested by phh (Reviewer).
PR: https://git.openjdk.org/jdk17u-dev/pull/811
More information about the jdk-updates-dev
mailing list