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

i#4550: Support EVEX encoded vsqrt instructions. #4739

Merged
merged 2 commits into from Feb 17, 2021
Merged

Conversation

khuey
Copy link
Contributor

@khuey khuey commented Feb 17, 2021

Adds support for the EVEX encodings of vsqrtps, vsqrtpd, vsqrtss, and vsqrtsd. Also adds API tests and uncomments the relevant sections of the binutils decoder tests, with updated expectation files.

Fixes #4550

Adds support for the EVEX encodings of vsqrtps, vsqrtpd, vsqrtss, and vsqrtsd. Also adds API tests and uncomments the relevant sections of the binutils decoder tests, with updated expectation files.

Fixes #4550
@khuey
Copy link
Contributor Author

khuey commented Feb 17, 2021

I don't think that test failure is real.

@derekbruening
Copy link
Contributor

I don't think that test failure is real.

The convention is to list the test name with a link to the filed issue (or to file a new one if convinced it's a flake).

Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this!

{OP_vsqrtpd, 0x660f5140, "vsqrtpd", Ved, xx, KEb, Wed, xx, mrm|evex|ttfv, x, modx[119][0]},
{MOD_EXT, 0x660f5150, "(mod ext 119)", xx, xx, xx, xx, xx, mrm|evex, x, 119},
}, { /* evex_W_ext 266 */
{OP_vsqrtss, 0xf30f5100, "vsqrtss", Vdq, xx, KE1b, Hdq, Wss, mrm|evex|ttt1s, x, tevexwb[266][1]},
Copy link
Contributor

Choose a reason for hiding this comment

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

(OK, tuple types of Full Vector and Typle 1 Scalar seem to match.)

@khuey
Copy link
Contributor Author

khuey commented Feb 17, 2021

I don't think that test failure is real.

The convention is to list the test name with a link to the filed issue (or to file a new one if convinced it's a flake).

I had trouble finding a useful error message in the log, other than just that decode-stress failed.

@derekbruening
Copy link
Contributor

I don't think that test failure is real.

The convention is to list the test name with a link to the filed issue (or to file a new one if convinced it's a flake).

I had trouble finding a useful error message in the log, other than just that decode-stress failed.

It seems to be there. The output is both above as verbose original and below as why it didn't match:

<Application /home/runner/work/dynamorio/dynamorio/build_debug-internal-32/suite/tests/bin/common.decode (19098).  Internal Error: DynamoRIO debug check failure: /home/runner/work/dynamorio/dynamorio/core/translate.c:1094 false

That is #4723.

@khuey
Copy link
Contributor Author

khuey commented Feb 17, 2021

I don't think that test failure is real.

The convention is to list the test name with a link to the filed issue (or to file a new one if convinced it's a flake).

I had trouble finding a useful error message in the log, other than just that decode-stress failed.

It seems to be there. The output is both above as verbose original and below as why it didn't match:

<Application /home/runner/work/dynamorio/dynamorio/build_debug-internal-32/suite/tests/bin/common.decode (19098).  Internal Error: DynamoRIO debug check failure: /home/runner/work/dynamorio/dynamorio/core/translate.c:1094 false

That is #4723.

Ah, great.

@khuey khuey merged commit df18aa8 into master Feb 17, 2021
@khuey khuey deleted the i4550-evex-vsqrt branch February 17, 2021 20:53
khuey added a commit to khuey/dynamorio that referenced this pull request Oct 21, 2022
The EVEX-prefixed vsqrtss/vsqrtsd entries should take
three-quarter and half width merge operands respectively, like
their VEX-prefixed counterparts.

This was an oversight when I wrote DynamoRIO#4739.
khuey added a commit that referenced this pull request Oct 22, 2022
The EVEX-prefixed vsqrtss/vsqrtsd entries should take
three-quarter and half width merge operands respectively, like
their VEX-prefixed counterparts.

This was an oversight when I wrote #4739.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EVEX encoded vsqrtps/vsqrtpd are missing
2 participants