-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8349138: Optimize Math.copySign API for Intel e-core targets #23386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d620eb6
2181850
a254873
d3c3f1d
ecc3658
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1538,10 +1538,7 @@ bool Matcher::match_rule_supported(int opcode) { | |||||
| break; | ||||||
| case Op_CopySignD: | ||||||
| case Op_CopySignF: | ||||||
| if (UseAVX < 3) { | ||||||
| return false; | ||||||
| } | ||||||
| if (!VM_Version::supports_avx512vl()) { | ||||||
| if (UseAVX < 1) { | ||||||
| return false; | ||||||
| } | ||||||
| break; | ||||||
|
|
@@ -1669,6 +1666,12 @@ bool Matcher::match_rule_supported_vector(int opcode, int vlen, BasicType bt) { | |||||
| return false; | ||||||
| } | ||||||
| break; | ||||||
| case Op_CopySignVD: | ||||||
| case Op_CopySignVF: | ||||||
| if (UseAVX < 1) { | ||||||
| return false; | ||||||
| } | ||||||
| break; | ||||||
| case Op_MaxV: | ||||||
| case Op_MinV: | ||||||
| if (UseSSE < 4 && is_integral_type(bt)) { | ||||||
|
|
@@ -6519,47 +6522,68 @@ instruct signumV_reg_evex(vec dst, vec src, vec zero, vec one, kReg ktmp1) %{ | |||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
| // --------------------------------------- | ||||||
| // For copySign use 0xE4 as writemask for vpternlog | ||||||
| // Desired Truth Table: A -> xmm0 bit, B -> xmm1 bit, C -> xmm2 bit | ||||||
| // C (xmm2) is set to 0x7FFFFFFF | ||||||
| // Wherever xmm2 is 0, we want to pick from B (sign) | ||||||
| // Wherever xmm2 is 1, we want to pick from A (src) | ||||||
| // | ||||||
| // A B C Result | ||||||
| // 0 0 0 0 | ||||||
| // 0 0 1 0 | ||||||
| // 0 1 0 1 | ||||||
| // 0 1 1 0 | ||||||
| // 1 0 0 0 | ||||||
| // 1 0 1 1 | ||||||
| // 1 1 0 1 | ||||||
| // 1 1 1 1 | ||||||
| // | ||||||
| // Result going from high bit to low bit is 0x11100100 = 0xe4 | ||||||
| // --------------------------------------- | ||||||
| #ifdef _LP64 | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _LP64 ifdef no more needed in .ad file (32 bit support has been removed). |
||||||
| instruct copySignF_reg(regF dst, regF src, regF xtmp) %{ | ||||||
| predicate(VM_Version::supports_avx512vl()); | ||||||
| match(Set dst (CopySignF dst src)); | ||||||
| effect(TEMP xtmp); | ||||||
| format %{ "CopySignF $dst, $src\t! using $xtmp as TEMP" %} | ||||||
| ins_encode %{ | ||||||
| __ vector_copy_sign_evex(T_FLOAT, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, Assembler::AVX_128bit); | ||||||
| %} | ||||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
| instruct copySignD_imm(regD dst, regD src, regD xtmp, immD zero) %{ | ||||||
| predicate(VM_Version::supports_avx512vl()); | ||||||
| match(Set dst (CopySignD dst (Binary src zero))); | ||||||
| ins_cost(100); | ||||||
| effect(TEMP xtmp); | ||||||
| format %{ "CopySignD $dst, $src\t! using $xtmp as TEMP" %} | ||||||
| ins_encode %{ | ||||||
| __ vector_copy_sign_evex(T_DOUBLE, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, Assembler::AVX_128bit); | ||||||
| %} | ||||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
| #endif // _LP64 | ||||||
|
|
||||||
| instruct copySignF_reg(regF dst, regF src, regF tmp1, rRegI tmp2) %{ | ||||||
| instruct copySignF_reg_avx(regF dst, regF src, regF xtmp) %{ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should be vlRegF. |
||||||
| predicate(!VM_Version::supports_avx512vl()); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Just to be a bit more explicit (and same for the one below).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its already handled by match_rule_supported contraint. |
||||||
| match(Set dst (CopySignF dst src)); | ||||||
| effect(TEMP tmp1, TEMP tmp2); | ||||||
| format %{ "CopySignF $dst, $src\t! using $tmp1 and $tmp2 as TEMP" %} | ||||||
| effect(TEMP_DEF dst,TEMP xtmp); | ||||||
| format %{ "CopySignF $dst, $src\t! using $xtmp as TEMP" %} | ||||||
| ins_encode %{ | ||||||
| __ movl($tmp2$$Register, 0x7FFFFFFF); | ||||||
| __ movdl($tmp1$$XMMRegister, $tmp2$$Register); | ||||||
| __ vpternlogd($dst$$XMMRegister, 0xE4, $src$$XMMRegister, $tmp1$$XMMRegister, Assembler::AVX_128bit); | ||||||
| __ vector_copy_sign_avx(T_FLOAT, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, Assembler::AVX_128bit); | ||||||
| %} | ||||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
| instruct copySignD_imm(regD dst, regD src, regD tmp1, rRegL tmp2, immD zero) %{ | ||||||
| instruct copySignD_imm_avx(regD dst, regD src, regD xtmp, immD zero) %{ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should be vlRegD. |
||||||
| predicate(!VM_Version::supports_avx512vl()); | ||||||
| match(Set dst (CopySignD dst (Binary src zero))); | ||||||
| ins_cost(100); | ||||||
| effect(TEMP tmp1, TEMP tmp2); | ||||||
| format %{ "CopySignD $dst, $src\t! using $tmp1 and $tmp2 as TEMP" %} | ||||||
| effect(TEMP_DEF dst,TEMP xtmp); | ||||||
| format %{ "CopySignD $dst, $src\t! using $xtmp as TEMP" %} | ||||||
| ins_encode %{ | ||||||
| __ vector_copy_sign_avx(T_DOUBLE, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, Assembler::AVX_128bit); | ||||||
| %} | ||||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
| instruct copySignV_reg(vec dst, vec src, vec xtmp) %{ | ||||||
| match(Set dst (CopySignVF dst src)); | ||||||
| match(Set dst (CopySignVD dst src)); | ||||||
| effect(TEMP xtmp); | ||||||
|
Comment on lines
+6574
to
+6577
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. vector_copy_sign_avx needs TEMP dst so may need two different instruct rules. |
||||||
| format %{ "vector_copysign $dst, $src\t! using $xtmp as TEMP" %} | ||||||
| ins_encode %{ | ||||||
| __ mov64($tmp2$$Register, 0x7FFFFFFFFFFFFFFF); | ||||||
| __ movq($tmp1$$XMMRegister, $tmp2$$Register); | ||||||
| __ vpternlogq($dst$$XMMRegister, 0xE4, $src$$XMMRegister, $tmp1$$XMMRegister, Assembler::AVX_128bit); | ||||||
| int vlen_enc = vector_length_encoding(this); | ||||||
| BasicType bt = Matcher::vector_element_basic_type(this); | ||||||
| if (VM_Version::supports_avx512vl() || Matcher::vector_length_in_bytes(this) == 64) { | ||||||
| __ vector_copy_sign_evex(bt, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, vlen_enc); | ||||||
| } else { | ||||||
| __ vector_copy_sign_avx(bt, $dst$$XMMRegister, $src$$XMMRegister, $xtmp$$XMMRegister, vlen_enc); | ||||||
| } | ||||||
| %} | ||||||
| ins_pipe( pipe_slow ); | ||||||
| %} | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AVX 1 supports 256-bit float/double vector and only128-bit vpsll, vpsrl, vpor for integer vectors. So you will have issues on AVX 1 platform for 256bit float/double vector copysign implementation using vpsll, vpsrl, vpor.