-
Notifications
You must be signed in to change notification settings - Fork 5k
[APX] Complete EGPR encoding in X64 backend #113237
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
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
The failures look irrelevant, PR is ready for review. |
Hi @tannergooding @BruceForstall This PR is ready for review, appreciate for any feedback. :) |
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.
LGTM
case INS_pdep: | ||
case INS_mulx: | ||
// TODO: Unblock when enabled for x86 | ||
#ifdef TARGET_AMD64 |
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.
Looks like this will get enabled by #111778, after which these ifdefs can be removed. Not sure which will get merged first.
/ba-g OSX infra issue |
Thanks for the quick response! |
With #106557 and #108796, we introduced APX new encodings - REX2 and extended EVEX, and with #108799, we introduced more general-purpose registers (EGPRs) in register allocator.
This PR addresses some left-over changes needed in the emitter to let both REX2 and extended EVEX address EGPRs properly.
Plus it introduced more APX compatible instructions.
Note that this PR only covers the encoding part and is not intended to let register allocator consider EGPR in more instructions/nodes. We are planning this part later. So this PR is expected to have no asmdiff with/without APX.