8354348: Enable Extended EVEX to REX2/REX demotion for commutative operations with same dst and src2 #26997
Conversation
…erations with same dst and src2
|
👋 Welcome back sparasa! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@vamsi-parasa The following label 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 list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
|
Hi @vamsi-parasa , thanks for working on this, I am process of validating #26283 and find that additional RA biasing will enable demotion for more cases, with a minimal test case I see following results Test point:- |
| bool normal_demotion = is_demotable(no_flags, dst_enc, nds_enc); | ||
| bool commutative_demotion = is_commutative && is_demotable(no_flags, dst_enc, src_enc); |
There was a problem hiding this comment.
Nomenclature change: instead of normal_demotion and commutative demotion, it will be more appropriate to use first/second_operand_demotable.
There was a problem hiding this comment.
Please see the updated nomenclature changed in the updated code as suggested.
| int size, int opcode_byte, bool no_flags, bool is_map1, bool is_commutative) { | ||
| if (is_commutative && is_demotable(no_flags, dst->encoding(), src2->encoding())) { | ||
| if (size == EVEX_64bit) { | ||
| emit_prefix_and_int8(get_prefixq(src1, dst, is_map1), opcode_byte + 2); |
There was a problem hiding this comment.
It will be good to write a comment on top of opcode_byte adjustment on account of opcode mismatch b/w NDD and equivalent demotable variant.
EVEX.LLZ.NP.MAP4.SCALABLE 21 /r AND {NF} {ND=1} rv, rv/mv, rv
REX.W + 23 /r AND r64, r/m64 | RM | Valid | N.E. | r64 AND r/m64
| instruct addI_rReg_mem_rReg_ndd(rRegI dst, memory src1, rRegI src2, rFlagsReg cr) | ||
| %{ | ||
| predicate(UseAPX); | ||
| match(Set dst (AddI (LoadI src1) src2)); |
There was a problem hiding this comment.
Will this not be covered by the pattern at line 7103, since ADLC automatically generates a DFA to handle both cases?
There was a problem hiding this comment.
Will run experiments to make sure that the RegRegMem pattern also applies to RegMemReg case and remove the newly added match rules if they're redundant. Will update you soon.
Hi Jatin (@jatin-bhateja), thank you for sharing the information about the register allocation biasing PR you're working on that will improve demotion. |
This change extends Extended EVEX (EEVEX) to REX2/REX demotion for Intel APX NDD instructions to handle commutative operations when the destination register and the second source register (src2) are the same.
Currently, EEVEX to REX2/REX demotion is only enabled when the first source (src1) and the destination are the same. This enhancement allows additional cases of valid demotion for commutative instructions (add, imul, and, or, xor).
For example:
eaddl r18, r25, r18can be encoded asaddl r18, r25using APX REX2 encodingeaddl r2, r7, r2can be encoded asaddl r2, r7using non-APX legacy encodingProgress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26997/head:pull/26997$ git checkout pull/26997Update a local copy of the PR:
$ git checkout pull/26997$ git pull https://git.openjdk.org/jdk.git pull/26997/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26997View PR using the GUI difftool:
$ git pr show -t 26997Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26997.diff
Using Webrev
Link to Webrev Comment