Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[RyuJIT] Fix VEX.vvvv for cvtss2sd and cvtsd2ss#14274

Merged
CarolEidt merged 1 commit into
dotnet:masterfrom
fiigii:vexencoding
Oct 2, 2017
Merged

[RyuJIT] Fix VEX.vvvv for cvtss2sd and cvtsd2ss#14274
CarolEidt merged 1 commit into
dotnet:masterfrom
fiigii:vexencoding

Conversation

@fiigii
Copy link
Copy Markdown

@fiigii fiigii commented Sep 30, 2017

Instruction cvtss2sd and cvtsd2ss has similar issue to #14225, which vcvtsd2ss xmm0, xmm1, xmm1 has less register dependency than vcvtsd2ss xmm0, xmm0, xmm1.

@fiigii
Copy link
Copy Markdown
Author

fiigii commented Oct 2, 2017

@CarolEidt @BruceForstall PTAL

@CarolEidt
Copy link
Copy Markdown

In looking at these a bit more (wondering why your previous change that included cvtsi2sd and cvtsi2ss in the DstSrcSrc category wasn't correct), it seems to me that only cvtss2si should remain in the DstDstSrc, and that cvtss2sd should be also moved to DstSrcSrc, as it has a 3-operand form that merges. Am I missing something?

@fiigii
Copy link
Copy Markdown
Author

fiigii commented Oct 2, 2017

@CarolEidt These two instructions were omitted in the previous PR. cvtsi2ss/d is different from cvtss2sd and cvtsd2ss.
In VEX-encoding format, vcvtsi2ss/d has different types on the second and the third operand: vcvtsi2ss xmm0, xmm0, rax, so we cannot eliminate the register dependency from the second operand xmm0.
However, cvtss2sd and cvtsd2ss has the same type on the second and the third operand: vcvtss2sd xmm0, xmm1, xmm2. So we can duplicate the src operand to avoid the additional register dependency.

vcvtss2sd xmm0, xmm0, xmm1 ;;; depends on xmm0 and xmm1
vcvtss2sd xmm0, xmm1, xmm1 ;;; depends on xmm1 only

@CarolEidt
Copy link
Copy Markdown

@fiigii - thanks! I knew there was something I was missing there.

Copy link
Copy Markdown

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM

@CarolEidt CarolEidt merged commit bd74934 into dotnet:master Oct 2, 2017
@fiigii fiigii deleted the vexencoding branch October 25, 2017 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants