-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[AMD64][APX] Introduce the remaining NCI instructions - CTEST, CFCMOV #127536
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
base: main
Are you sure you want to change the base?
Changes from all commits
d0d4999
849f26e
c181556
c958535
e45ac57
ffa4778
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1436,6 +1436,7 @@ instruction CodeGen::JumpKindToCmov(emitJumpKind condition) | |||||||||||
| return s_table[condition]; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #ifdef TARGET_AMD64 | ||||||||||||
| //------------------------------------------------------------------------ | ||||||||||||
| // JumpKindToCcmp: | ||||||||||||
| // Convert an emitJumpKind to the corresponding ccmp instruction. | ||||||||||||
|
|
@@ -1475,6 +1476,7 @@ instruction CodeGen::JumpKindToCcmp(emitJumpKind condition) | |||||||||||
| assert((condition >= EJ_NONE) && (condition < EJ_COUNT)); | ||||||||||||
| return s_table[condition]; | ||||||||||||
| } | ||||||||||||
| #endif // TARGET_AMD64 | ||||||||||||
|
|
||||||||||||
| //------------------------------------------------------------------------ | ||||||||||||
| // genCodeForCompare: Produce code for a GT_SELECT/GT_SELECTCC node. | ||||||||||||
|
|
@@ -8631,8 +8633,9 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize, | |||||||||||
| regSet.verifyRegistersUsed(killMask); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #ifdef TARGET_AMD64 | ||||||||||||
| //----------------------------------------------------------------------------------------- | ||||||||||||
| // OptsFromCFlags - Convert condition flags into approxpriate insOpts. | ||||||||||||
| // OptsFromCFlags - Convert condition flags into appropriate insOpts. | ||||||||||||
| // | ||||||||||||
| // Arguments: | ||||||||||||
| // flags - The condition flags to be converted. | ||||||||||||
|
|
@@ -8642,7 +8645,7 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize, | |||||||||||
| // | ||||||||||||
| // Notes: | ||||||||||||
| // This function maps the condition flags (e.g., CF, ZF, SF, OF) to the appropriate | ||||||||||||
| // instruction options used for setting the default flag values in extneded EVEX | ||||||||||||
| // instruction options used for setting the default flag values in extended EVEX | ||||||||||||
| // encoding conditional instructions. | ||||||||||||
| // | ||||||||||||
| insOpts CodeGen::OptsFromCFlags(insCflags flags) | ||||||||||||
|
|
@@ -8659,8 +8662,6 @@ insOpts CodeGen::OptsFromCFlags(insCflags flags) | |||||||||||
| return (insOpts)opts; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| #ifdef TARGET_AMD64 | ||||||||||||
|
|
||||||||||||
| //----------------------------------------------------------------------------------------- | ||||||||||||
| // genCodeForCCMP - Generate code for a conditional compare (CCMP) node. | ||||||||||||
| // | ||||||||||||
|
|
@@ -8699,7 +8700,17 @@ void CodeGen::genCodeForCCMP(GenTreeCCMP* ccmp) | |||||||||||
| if (op2->isContainedIntOrIImmed()) | ||||||||||||
| { | ||||||||||||
| GenTreeIntConCommon* intConst = op2->AsIntConCommon(); | ||||||||||||
| emit->emitIns_R_I(ccmpIns, cmpSize, srcReg1, (int)intConst->IconValue(), opts); | ||||||||||||
| if (intConst->IconValue() == 0) | ||||||||||||
| { | ||||||||||||
| // ctest reg, reg is 1-byte shorter encoding than ccmp reg, 0. | ||||||||||||
| assert((FIRST_CTEST_INSTRUCTION - FIRST_APX_INSTRUCTION) == 32); | ||||||||||||
| instruction ctestIns = (instruction)(ccmpIns + FIRST_CTEST_INSTRUCTION - FIRST_CCMP_INSTRUCTION); | ||||||||||||
|
Comment on lines
+8706
to
+8707
|
||||||||||||
| assert((FIRST_CTEST_INSTRUCTION - FIRST_APX_INSTRUCTION) == 32); | |
| instruction ctestIns = (instruction)(ccmpIns + FIRST_CTEST_INSTRUCTION - FIRST_CCMP_INSTRUCTION); | |
| constexpr int ccmpToCtestOffset = FIRST_CTEST_INSTRUCTION - FIRST_CCMP_INSTRUCTION; | |
| static_assert(ccmpToCtestOffset == 32); | |
| instruction ctestIns = static_cast<instruction>(ccmpIns + ccmpToCtestOffset); |
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.
Fix proposed here: #127536 (comment)
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.
Deriving
ctestInsvia instruction-enum arithmetic is brittle (it silently depends on the relative ordering and contiguity of CCMP and CTEST instruction enums). Prefer an explicit mapping helper (e.g., deriveinsCConce and map to the corresponding CTEST instruction via a small table/switch) so future instruction table edits don’t break this transformation.Uh oh!
There was an error while loading. Please reload this page.
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.
added an assert to make sure the offset between
FIRST_CCMP_INSTRUCTIONandFIRST_CTEST_INSTRUCTIONis not changed. This is a simply, 1:1 mapping, probably not necessarily to use a switch table.But I am open to change the design if other reviewers are inclined to enhance the robustness with the switch table.