i#7410: Fix incorrect x86 ISA categories and movntdqa operands #7411
Conversation
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
Adds a check that the load and store categories strictly match the
instr_{reads,writes}_memory() on every x86 opcode. This revealed a
number of incorrect catLoad and catStore labels which are now removed
from:
+ OP_lea (see also below)
+ OP_lahf (see also below)
+ OP_vbroadcastf32x2
+ OP_vbroadcasti32x2
+ OP_fld with only register operands
+ OP_fst{,p} with only register operands
+ OP_fld* which insert literals
Fixes {v,}movntdqa which the above check detected had its operands in
the wrong order: it was listed as a store in the table when it is in
fact a load. Also adds catSIMD to some variants that were missing it.
Fixes the following x86 ISA category errors:
+ OP_lea is now catMath instead of catLoad
+ OP_nop_modrm is now catOther instead of catSIMD
+ OP_lahf is now catState instead of catLoad
Categorizes the following previously uncategorized opcodes:
+ OP_xchg and OP_fxchg are now catMove
+ OP_pushf is now catStore|catState
+ OP_popf is now catLoad|catState
+ OP_sahf is now catState
+ OP_cmov* is now catMove
+ OP_nop is now catOther
Fixes #7410
|
I see you started a review: there was one test failure: had to update opcode_categories output (that asm has OP_lea in it); all set now. |
edeiana
approved these changes
Mar 28, 2025
|
VS2019-32 failure is: Yet is was green in the first commit. So on the borderline? May have to split the ir test headers further? |
|
Now record_filter_unit_tests has a failure that may be related: but it didn't fail in the 1st commit? |
…s_avx512_evex_mask_B.h to avoid OOM in VS (xref i#3992,i#4610)
derekbruening
added a commit
that referenced
this pull request
Apr 1, 2025
Adds support for "-mode regdeps" in all builds of drdisas. I found this useful when working on PR #7411 where I had to update/understand regdeps disassembly in our tests. Adds a test. Also manually tested: ``` $ clients/bin64/drdisas -mode regdeps 00001931 04020204 00000026 00001931 04020204 load store [4byte] %rv0 %rv2 %rv36 -> %rv0 00000026 ``` Issue: #6662, #7410
derekbruening
added a commit
that referenced
this pull request
Apr 1, 2025
Adds support for "-mode regdeps" in all builds of drdisas. I found this useful when working on PR #7411 where I had to update/understand regdeps disassembly in our tests. Adds a test. Also manually tested: ``` $ clients/bin64/drdisas -mode regdeps 00001931 04020204 00000026 00001931 04020204 load store [4byte] %rv0 %rv2 %rv36 -> %rv0 00000026 ``` Issue: #6662, #7410
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds a check that the load and store categories strictly match the instr_{reads,writes}_memory() on every x86 opcode. This revealed a number of incorrect catLoad and catStore labels which are now removed from:
Fixes {v,}movntdqa which the above check detected had its operands in the wrong order: it was listed as a store in the table when it is in fact a load. Also adds catSIMD to some variants that were missing it.
Fixes the following x86 ISA category errors:
Categorizes the following previously uncategorized opcodes:
None of these changes affect other IR interfaces like instr_{reads,write}_memory().
There is no compatibility issue with drmemtrace address traces: no raw trace impact; the final trace is impacted for movntdqa and (for regdeps) the categories but there's no compatibility change: just an increase in accuracy from raw2trace or record_filter output.
Fixes #7410