Skip to content
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

Update xarch to track more instruction metadata as flags #83473

Merged
merged 11 commits into from
Mar 19, 2023

Conversation

tannergooding
Copy link
Member

@tannergooding tannergooding commented Mar 15, 2023

This allows us to remove several large switch tables and accelerates the general lookup case to help ensure we can easily determine whether a given instruction supports the kmask or other extended SIMD registers in the first place.

I recommend reviewing this with "Hide Whitespace" and commit by commit. The first commit in particular is a no-op and just reorders the instructions (see below) so reviewing by commit allows you to more easily see the actual changes which are just flags additions.


The order of the SIMD instructions was redone to be grouped alphabetically by ISA. This assisted in the ability to find and match things in the architecture manuals for correctness.

We now track whether a given instruction supports the VEX or EVEX encodings.

We also track whether an instruction has an alternative VEX or EVEX instruction with a different encoding and which can therefore support VEX/EVEX but needs specialized handling in codegen or emit to do so. This ended up being very error prone overall and it was overall simpler to just handle the difference in the spot that directly needed it (namely disasm output).

We now track whether a given instruction encodes REX.W as 0, 1, or X meaning "variable". We also define a helper WIG flag that has the same value as W0, but which allows us to easily take advantage of this information in the future if necessary.

It's worth noting that for many instructions, they are REX.WIG for the VEX encoding and may strictly require REX.W0 or REX.W1 for the EVEX encoding. In such scenarios, the instruction preferenced the REX.W0 or REX.W1 EVEX requirement to simplify things since this will result in a still valid VEX encoding without requiring the specialized handling or another flag to differentiate. We do actually need to track REX_W1_EVEX to ensure that we can still emit the 2-byte VEX encoding when possible when the VEX encoding is WIG

@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI new-api-needs-documentation labels Mar 15, 2023
@ghost ghost assigned tannergooding Mar 15, 2023
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Mar 15, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

This depends on #83354

Author: tannergooding
Assignees: tannergooding
Labels:

area-CodeGen-coreclr, new-api-needs-documentation

Milestone: -

@tannergooding
Copy link
Member Author

tannergooding commented Mar 17, 2023

CC. @dotnet/jit-contrib, @dotnet/avx512-contrib this should be ready for review now.

Please see the top post for an explanation of the changes and recommended review process given the first commit is a no-op reordering of instruction entries.

@tannergooding
Copy link
Member Author

Hmmm, size is being miscomputed somewhere based on the new metadata, even where it doesn't appear to change the output assembly. Looking...

Copy link
Contributor

@TIHan TIHan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, this looks good; will wait till you sort out the failures.

@tannergooding
Copy link
Member Author

Hmmm, size is being miscomputed somewhere based on the new metadata, even where it doesn't appear to change the output assembly. Looking...

I forgot that REX.W = 1 requires the 3-byte VEX encoding, so I had to update the metadata flags to disambiguate REX_W1 vs REX_W1_EVEX, the latter indicating it is W1 for EVEX and WIG or W0 otherwise

@tannergooding tannergooding merged commit 9c818a3 into dotnet:main Mar 19, 2023
@tannergooding tannergooding deleted the REX_W branch March 19, 2023 14:30
@ghost ghost locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants