-
-
Notifications
You must be signed in to change notification settings - Fork 438
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
Add APX
ISA extension
#450
base: master
Are you sure you want to change the base?
Conversation
flobernd
commented
Sep 12, 2023
•
edited
Loading
edited
@@ -88,6 +88,12 @@ ZyanStatus ZydisGetInstructionSegments(const ZydisDecodedInstruction* instructio | |||
segment_size = 4; | |||
break; | |||
default: | |||
if (instruction->attributes & ZYDIS_ATTRIB_HAS_REX2) |
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.
This should check encoding instead.
@@ -777,6 +810,10 @@ static void PrintAVXInfo(const ZydisDecodedInstruction* instruction) | |||
ZydisRegisterGetString(instruction->avx.mask.reg), | |||
CVT100_OUT(COLOR_VALUE_LABEL), CVT100_OUT(COLOR_VALUE_B), | |||
strings_mask_mode[instruction->avx.mask.mode], CVT100_OUT(COLOR_VALUE_LABEL)); | |||
if (instruction->attributes & ZYDIS_ATTRIB_HAS_SCC) |
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.
We should probably put this in a dedicated APX section together with the "zu" (which is not implemented yet at all).
@@ -2351,6 +2402,8 @@ static void ZydisSetAVXInformation(ZydisDecoderContext* context, | |||
const ZydisInstructionDefinitionEVEX* def = | |||
(const ZydisInstructionDefinitionEVEX*)definition; | |||
|
|||
// TODO: Don't set vector length, etc. for scalar instructions |
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.
This requires a dedicated flag and generator support.
break; | ||
} | ||
|
||
ZyanBool illegal_rex2 = (instruction->opcode == 0x0F); |
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.
Might be better to just generate a flag for this in the generator based on the same condition. We usually try to not hardcode table related conditions.
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.
There are as well XSAVE and XRSTORE variants that do not support REX2.
(instruction->opcode_map == ZYDIS_OPCODE_MAP_MAP4) && | ||
(instruction->raw.evex.pp == 0x01)) | ||
{ | ||
// EVEX encoded instructions in MAP4 must use 0x66 as the mandatory prefix AND the |
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.
Might be better to have a flag for this. We usually don't hardcode table related conditions.
return ZYDIS_STATUS_DECODING_ERROR; | ||
} | ||
|
||
// TODO: NF without ND filter -> evex.b = zero upper |
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.
Generator flag would be better here.
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.
EVEX.b is actually EVEX.ND in these cases.
|
||
// All APX conditional CMP and TEST instructions must have a SCC filter! | ||
|
||
context->vector_unified.vvvv = (~context->vector_unified.vvvv) & 0x0F; |
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.
This is clumsy.. It would be nice if the generator emits a hint about the EVEX variant to use. We are currently just "guessing".
@@ -1009,6 +1053,7 @@ static void ZydisSetOperandSizeAndElementInfo(const ZydisDecoderContext* context | |||
#ifndef ZYDIS_DISABLE_AVX512 | |||
if (definition->size[context->eosz_index]) | |||
{ | |||
// TODO: fixme |
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.
This block is not entered for some tile instructions, resulting in a panic.