Enable EVEX feature: Embedded Rounding for Avx512F.Add() #94684
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1139,6 +1139,29 @@ static bool isLowSimdReg(regNumber reg) | |||||
| #endif | ||||||
| } | ||||||
|
|
||||||
| //------------------------------------------------------------------------ | ||||||
| // GetEmbRoudingMode: Get the rounding mode for embedded rounding | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| // | ||||||
| // Arguments: | ||||||
| // mode -- the flag from the correspoding gentree node indicating the mode. | ||||||
|
There was a problem hiding this comment. nit (typo)
Suggested change
|
||||||
| // | ||||||
| // Return Value: | ||||||
| // the instruction option carrying the rounding mode information. | ||||||
|
There was a problem hiding this comment. nit: in general, we prefer to have a blank |
||||||
| insOpts emitter::GetEmbRoundingMode(uint8_t mode) const | ||||||
| { | ||||||
| switch (mode) | ||||||
| { | ||||||
| case 1: | ||||||
| return INS_OPTS_EVEX_eb_er_rd; | ||||||
| case 2: | ||||||
| return INS_OPTS_EVEX_er_ru; | ||||||
| case 3: | ||||||
| return INS_OPTS_EVEX_er_rz; | ||||||
| default: | ||||||
| unreached(); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| //------------------------------------------------------------------------ | ||||||
| // encodeRegAsIval: Encodes a register as an ival for use by a SIMD instruction | ||||||
| // | ||||||
|
|
@@ -1309,18 +1332,50 @@ emitter::code_t emitter::AddEvexPrefix(const instrDesc* id, code_t code, emitAtt | |||||
|
|
||||||
| if (attr == EA_32BYTE) | ||||||
| { | ||||||
| // Set L bit to 01 in case of instructions that operate on 256-bits. | ||||||
|
There was a problem hiding this comment. You write "bit" (singular) but give an example of 2 bits. Should it either be left alone (as it was), or:
Suggested change
|
||||||
| code |= LBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| } | ||||||
| else if (attr == EA_64BYTE) | ||||||
| { | ||||||
| // Set L' bits to 10 in case of instructions that operate on 512-bits. | ||||||
|
There was a problem hiding this comment. Similar to above: should it say L'L?
Suggested change
|
||||||
| code |= LPRIMEBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| } | ||||||
|
|
||||||
| if (id->idIsEvexbContext()) | ||||||
| { | ||||||
| code |= EVEX_B_BIT; | ||||||
|
|
||||||
| if (!id->idHasMem()) | ||||||
| { | ||||||
| // embedded rounding case. | ||||||
| unsigned roundingMode = id->idGetEvexbContext(); | ||||||
| if (roundingMode == 1) | ||||||
| { | ||||||
| // {rd-sae} | ||||||
| code &= ~(LPRIMEBIT_IN_BYTE_EVEX_PREFIX); | ||||||
| code |= LBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| } | ||||||
| else if (roundingMode == 2) | ||||||
| { | ||||||
| // {ru-sae} | ||||||
| code |= LPRIMEBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| code &= ~(LBIT_IN_BYTE_EVEX_PREFIX); | ||||||
| } | ||||||
| else if (roundingMode == 3) | ||||||
| { | ||||||
| // {rz-sae} | ||||||
| code |= LPRIMEBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| code |= LBIT_IN_BYTE_EVEX_PREFIX; | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| unreached(); | ||||||
| } | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| assert(id->idGetEvexbContext() == 1); | ||||||
| } | ||||||
tannergooding marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
|
|
||||||
| regNumber maskReg = REG_NA; | ||||||
|
|
@@ -6734,11 +6789,7 @@ void emitter::emitIns_R_R_A( | |||||
| id->idIns(ins); | ||||||
| id->idReg1(reg1); | ||||||
| id->idReg2(reg2); | ||||||
| SetEvexBroadcastIfNeeded(id, instOptions); | ||||||
|
|
||||||
| emitHandleMemOp(indir, id, (ins == INS_mulx) ? IF_RWR_RWR_ARD : emitInsModeFormat(ins, IF_RRD_RRD_ARD), ins); | ||||||
|
|
||||||
|
|
@@ -6863,11 +6914,7 @@ void emitter::emitIns_R_R_C(instruction ins, | |||||
| id->idReg1(reg1); | ||||||
| id->idReg2(reg2); | ||||||
| id->idAddr()->iiaFieldHnd = fldHnd; | ||||||
| SetEvexBroadcastIfNeeded(id, instOptions); | ||||||
|
|
||||||
| UNATIVE_OFFSET sz = emitInsSizeCV(id, insCodeRM(ins)); | ||||||
| id->idCodeSize(sz); | ||||||
|
|
@@ -6881,7 +6928,8 @@ void emitter::emitIns_R_R_C(instruction ins, | |||||
| * Add an instruction with three register operands. | ||||||
| */ | ||||||
|
|
||||||
| void emitter::emitIns_R_R_R( | ||||||
| instruction ins, emitAttr attr, regNumber targetReg, regNumber reg1, regNumber reg2, insOpts instOptions) | ||||||
| { | ||||||
| assert(IsAvx512OrPriorInstruction(ins)); | ||||||
| assert(IsThreeOperandAVXInstruction(ins) || IsKInstruction(ins)); | ||||||
|
|
@@ -6893,6 +6941,13 @@ void emitter::emitIns_R_R_R(instruction ins, emitAttr attr, regNumber targetReg, | |||||
| id->idReg2(reg1); | ||||||
| id->idReg3(reg2); | ||||||
|
|
||||||
| if ((instOptions & INS_OPTS_b_MASK) != INS_OPTS_NONE) | ||||||
| { | ||||||
| // if EVEX.b needs to be set in this path, then it should be embedded rounding. | ||||||
| assert(UseEvexEncoding()); | ||||||
| id->idSetEvexbContext(instOptions); | ||||||
| } | ||||||
|
|
||||||
| UNATIVE_OFFSET sz = emitInsSizeRR(id, insCodeRM(ins)); | ||||||
| id->idCodeSize(sz); | ||||||
|
|
||||||
|
|
@@ -6913,12 +6968,8 @@ void emitter::emitIns_R_R_S( | |||||
| id->idReg1(reg1); | ||||||
| id->idReg2(reg2); | ||||||
| id->idAddr()->iiaLclVar.initLclVarAddr(varx, offs); | ||||||
| SetEvexBroadcastIfNeeded(id, instOptions); | ||||||
|
|
||||||
| #ifdef DEBUG | ||||||
| id->idDebugOnlyInfo()->idVarRefOffs = emitVarRefOffs; | ||||||
| #endif | ||||||
|
|
@@ -8216,11 +8267,11 @@ void emitter::emitIns_SIMD_R_R_C(instruction ins, | |||||
| // op2Reg -- The register of the second operand | ||||||
| // | ||||||
| void emitter::emitIns_SIMD_R_R_R( | ||||||
| instruction ins, emitAttr attr, regNumber targetReg, regNumber op1Reg, regNumber op2Reg, insOpts instOptions) | ||||||
| { | ||||||
| if (UseSimdEncoding()) | ||||||
| { | ||||||
| emitIns_R_R_R(ins, attr, targetReg, op1Reg, op2Reg, instOptions); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
|
|
@@ -10657,6 +10708,37 @@ void emitter::emitDispEmbBroadcastCount(instrDesc* id) | |||||
| printf(" {1to%d}", vectorSize / baseSize); | ||||||
| } | ||||||
|
|
||||||
| // emitDispEmbRounding: Display the tag where embedded rounding is activated | ||||||
| // | ||||||
| // Arguments: | ||||||
| // id - The instruction descriptor | ||||||
| // | ||||||
| void emitter::emitDispEmbRounding(instrDesc* id) | ||||||
| { | ||||||
| if (!id->idIsEvexbContext()) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
| assert(!id->idHasMem()); | ||||||
| unsigned roundingMode = id->idGetEvexbContext(); | ||||||
| if (roundingMode == 1) | ||||||
| { | ||||||
| printf(" {rd-sae}"); | ||||||
|
There was a problem hiding this comment. nit: in the Intel manual I have, section "15.6.4 Static Rounding Mode and Suppress All Exceptions", there are these examples: In each case there is a comma before the It's not clear from the instruction set reference whether the comma is expected or not. So what is the standard? Comma or no comma? |
||||||
| } | ||||||
| else if (roundingMode == 2) | ||||||
| { | ||||||
| printf(" {ru-sae}"); | ||||||
| } | ||||||
| else if (roundingMode == 3) | ||||||
| { | ||||||
| printf(" {rz-sae}"); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| unreached(); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| //-------------------------------------------------------------------- | ||||||
| // emitDispIns: Dump the given instruction to jitstdout. | ||||||
| // | ||||||
|
|
@@ -11525,6 +11607,7 @@ void emitter::emitDispIns( | |||||
| printf("%s, ", emitRegName(id->idReg1(), attr)); | ||||||
| printf("%s, ", emitRegName(reg2, attr)); | ||||||
| printf("%s", emitRegName(reg3, attr)); | ||||||
| emitDispEmbRounding(id); | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,6 +157,8 @@ bool IsRedundantCmp(emitAttr size, regNumber reg1, regNumber reg2); | |
| bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition cond); | ||
| bool AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenCondition cond); | ||
|
|
||
| insOpts GetEmbRoundingMode(uint8_t mode) const; | ||
|
|
||
| bool hasRexPrefix(code_t code) | ||
| { | ||
| #ifdef TARGET_AMD64 | ||
|
|
@@ -335,6 +337,25 @@ code_t AddSimdPrefixIfNeeded(const instrDesc* id, code_t code, emitAttr size) | |
| return code; | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // SetEvexBroadcastIfNeeded: set embedded broadcast if needed. | ||
| // | ||
| // Arguments: | ||
| // id - instruction descriptor | ||
| // instOptions - emit options | ||
| void SetEvexBroadcastIfNeeded(instrDesc* id, insOpts instOptions) | ||
| { | ||
| if ((instOptions & INS_OPTS_b_MASK) == INS_OPTS_EVEX_eb_er_rd) | ||
| { | ||
| assert(UseEvexEncoding()); | ||
| id->idSetEvexbContext(instOptions); | ||
| } | ||
| else | ||
| { | ||
| assert(instOptions == 0); | ||
| } | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // AddSimdPrefixIfNeeded: Add the correct SIMD prefix. | ||
| // Check if the prefix already exists befpre adding. | ||
|
|
@@ -627,7 +648,12 @@ void emitIns_R_R_S(instruction ins, | |
| int offs, | ||
| insOpts instOptions = INS_OPTS_NONE); | ||
|
|
||
| void emitIns_R_R_R(instruction ins, | ||
| emitAttr attr, | ||
| regNumber reg1, | ||
| regNumber reg2, | ||
| regNumber reg3, | ||
| insOpts instOptions = INS_OPTS_NONE); | ||
|
|
||
| void emitIns_R_R_A_I( | ||
| instruction ins, emitAttr attr, regNumber reg1, regNumber reg2, GenTreeIndir* indir, int ival, insFormat fmt); | ||
|
|
@@ -738,7 +764,12 @@ void emitIns_SIMD_R_R_C(instruction ins, | |
| CORINFO_FIELD_HANDLE fldHnd, | ||
| int offs, | ||
| insOpts instOptions = INS_OPTS_NONE); | ||
| void emitIns_SIMD_R_R_R(instruction ins, | ||
| emitAttr attr, | ||
| regNumber targetReg, | ||
| regNumber op1Reg, | ||
| regNumber op2Reg, | ||
| insOpts instOptions = INS_OPTS_NONE); | ||
| void emitIns_SIMD_R_R_S(instruction ins, | ||
| emitAttr attr, | ||
| regNumber targetReg, | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Missing a semicolon (probably got lucky that there is a "stray" semicolon below on
CLANG_FORMAT_COMMENT_ANCHORAlso, "Does Evex.b need to be set" makes it sound like this is a 0/1, false/true boolean. Then why does it need 2 bits? I think you should expand the comment to state what the two bits are used for. (You don't need to put the comment all on this one line; make it a multi-line comment above this line if necessary)
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.
Thanks for the comments!
_idEvexbContextis used to differentiate several features: normal intrinsics, embedded broadcast, embedded rounding. In the normal or embedded broadcast cases, the semantic forEvex.L'Lremains the same, intrinsic vector length, while in embedded rounding case, the semantic changes to indicate the rounding mode._idEvexbContextwill be used to inform emitter to specially handle theEvex.L'Lin the embedded rounding scenario. I will properly document this part in the comments.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.
A comment like you just wrote would be great.