8273322: Enhance macro logic optimization for masked logic operations. #6893
Conversation
|
👋 Welcome back jbhateja! A progress list of the required criteria for merging this PR into |
|
/label add hotspot-compiler-dev |
|
@jatin-bhateja The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
openjdk
bot
added
hotspot-compiler
|
@jatin-bhateja The |
Webrevs
|
vnkozlov
left a comment
vnkozlov
left a comment
There was a problem hiding this comment.
I think whole "Bitwise operation packing optimization" code should be moved out from compile.cpp. May be to vectornode.cpp where MacroLogicVNode` code is located.
Copyright year should be updated to 2022 in all changed files.
| return true; | ||
|
|
||
| case Op_MacroLogicV: | ||
| if(bt != T_INT && bt != T_LONG) { |
There was a problem hiding this comment.
Missing VM_Version::supports_evex() check?
There was a problem hiding this comment.
Hi @vnkozlov, we already have that check (UseAVX < 3) in match_rule_supported routine which gets called from this function.
vnkozlov
left a comment
vnkozlov
left a comment
There was a problem hiding this comment.
Let me test it before approval. You need second review.
And file RFE to move vector code from compile.cpp. Let do it separately from these changes.
|
|
src/hotspot/cpu/x86/x86.ad
Outdated
| match(Set dst (MacroLogicV dst (Binary src2 (Binary src3 (Binary func mask))))); | ||
| format %{ "vternlog_masked $dst,$src2,$src3,$func,$mask\t! vternlog masked operation" %} | ||
| ins_encode %{ | ||
| int vector_len = vector_length_encoding(this); |
There was a problem hiding this comment.
It would be good to name this as vlen_enc instead of vector_len.
| void C2_MacroAssembler::evpternlog(XMMRegister dst, int func, KRegister mask, XMMRegister src2, Address src3, | ||
| bool merge, BasicType bt, int vlen_enc) { | ||
| if (bt == T_INT) { | ||
| evpternlogd(dst, func, mask, src2, src3, true, vlen_enc); | ||
| } else { | ||
| assert(bt == T_LONG, ""); | ||
| evpternlogq(dst, func, mask, src2, src3, true, vlen_enc); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
"merge" argument is not used in the method body.
| void C2_MacroAssembler::evpternlog(XMMRegister dst, int func, KRegister mask, XMMRegister src2, XMMRegister src3, | ||
| bool merge, BasicType bt, int vlen_enc) { | ||
| if (bt == T_INT) { | ||
| evpternlogd(dst, func, mask, src2, src3, true, vlen_enc); | ||
| } else { | ||
| assert(bt == T_LONG, ""); | ||
| evpternlogq(dst, func, mask, src2, src3, true, vlen_enc); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
"merge" argument not used in method body.
| emit_int8(0x25); | ||
| emit_int8((unsigned char)(0xC0 | encode)); | ||
| emit_int8(imm8); |
| emit_int8(0x25); | ||
| emit_int8((unsigned char)(0xC0 | encode)); | ||
| emit_int8(imm8); |
Hi @vnkozlov ,
|
|
@jatin-bhateja This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 36 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
|
||
| /** | ||
| * @test | ||
| * @bug 8273322 |
There was a problem hiding this comment.
Needs @key randomness as we use random number without a fixed seed here.
Please see:
https://openjdk.java.net/jtreg/faq.html#when-should-i-use-the-intermittent-or-randomness-keyword-in-a-test
|
/integrate |
|
Going to push as commit 8703f14.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
|
@jatin-bhateja Pushed as commit 8703f14. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
3 participants
Patch extends existing macrologic inferencing algorithm to handle masked logic operations.
Existing algorithm:
i.e. maximum number of inputs which a macro logic node can have.
to each input.
Modification:
Extended the packing algorithm to operate on both predicated or non-predicated logic nodes. Following
rules define the criteria under which nodes gets packed into a macro logic node:-
New jtreg test case added with the patch exhaustively covers all the different combinations of predications of parent and
child nodes.
Following are the performance number for JMH benchmark included with the patch.
Machine Configuration: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (40C 2S Icelake Server)
Kindly review and share your feedback.
Best Regards,
Jatin
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6893/head:pull/6893$ git checkout pull/6893Update a local copy of the PR:
$ git checkout pull/6893$ git pull https://git.openjdk.java.net/jdk pull/6893/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 6893View PR using the GUI difftool:
$ git pr show -t 6893Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6893.diff