Skip to content

Do not eliminate casts from FP types when optimizing narrow stores - #53667

Merged
kunalspathak merged 1 commit into
dotnet:mainfrom
SingleAccretion:Fix-Potential-Issue-With-Cast-Elimination-For-Stores
Jul 13, 2021
Merged

Do not eliminate casts from FP types when optimizing narrow stores#53667
kunalspathak merged 1 commit into
dotnet:mainfrom
SingleAccretion:Fix-Potential-Issue-With-Cast-Elimination-For-Stores

Conversation

@SingleAccretion

@SingleAccretionSingleAccretion commented Jun 3, 2021

Copy link
Copy Markdown
Contributor

An optimization in morph was deleting casts on the RHS of a narrow store if the cast-to-type
was wider than the type being stored. This is only valid for casts from integral types, as the backend
does not handle STOREIND(byte*, double), nor is there an instruction to go from an XMM register to a narrow memory location on x86/x64.

The issue is not reproducible right now because fgMorphCast wraps the casts in question, but it is an invalid IR transformation nonetheless, and similar code in fgMorphSmpOpOptional guards against non-integral sources.

Also, while I was there, modernized the code a little bit.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 3, 2021
@SingleAccretion
SingleAccretionforce-pushed the Fix-Potential-Issue-With-Cast-Elimination-For-Stores branch from ff54d81 to 4dd49bbCompareJune 3, 2021 09:36
@SingleAccretion
SingleAccretion marked this pull request as ready for review June 3, 2021 14:47
An optimization in morph was deleting casts
on the RHS of a narrow store if the cast-to-type
was wider than the type being stored. This is only
valid for casts from integral types, as the backend
does not handle "STOREIND(byte*, double)", nor is there
an instruction to go from an XMM register to a narrow
memory location on x86/x64.
The issue is not reproducible right now because
fgMorphCast wraps the casts in question, but it is
an invalid IR transformation nonetheless, and similar
code in fgMorphSmpOpOptional guards against non-integral sources.
@SingleAccretion
SingleAccretionforce-pushed the Fix-Potential-Issue-With-Cast-Elimination-For-Stores branch from 4dd49bb to 94de26aCompareJune 7, 2021 20:50
@tannergooding

Copy link
Copy Markdown
Member

CC. @dotnet/jit-contrib, community PR

@JulieLeeMSFT

Copy link
Copy Markdown
Member

@kunalspathak @dotnet/jit-contrib PTAL community PR.

@kunalspathak

Copy link
Copy Markdown
Contributor

@SingleAccretion - No asmdiffs?

@SingleAccretion

Copy link
Copy Markdown
ContributorAuthor

No asmdiffs?

Yes, no diffs as expected.

@kunalspathakkunalspathak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@kunalspathak
kunalspathak merged commit 53db2ec into dotnet:mainJul 13, 2021
@SingleAccretion
SingleAccretion deleted the Fix-Potential-Issue-With-Cast-Elimination-For-Stores branch July 13, 2021 16:49
@ghostghost locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR 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.

4 participants

@SingleAccretion@tannergooding@JulieLeeMSFT@kunalspathak