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

Conversation

sivarv
Copy link
Member

@sivarv sivarv commented Feb 8, 2016

JitStress=1/2 is inducing GS check as a result of which the Vector argument gets shadow copied and all occurrences of original arg gets replaced by its shadow copy lcl var. But on the shadow copy lcl var SIMD specific state is not set and as a result of which rationalization doesn't happen properly leading to silent bad codegen.

This fixes half of issue #2886.

JitStress=1/2 is inducing GS check as a result of which the Vector argument
gets shadow copied.  But on the shadow copy lcl var SIMD specific state
is not set and as a result of which rationalization doesn't happen
properly leading to silent bad codegen.
@sivarv
Copy link
Member Author

sivarv commented Feb 8, 2016

@dotnet/jit-contrib - please review this.

@erozenfeld
Copy link
Member

LGTM

@sivarv
Copy link
Member Author

sivarv commented Feb 8, 2016

@dotnet-bot test this please

@sivarv sivarv added the os-linux Linux OS (any supported distro) label Feb 8, 2016

var_types type = varTypeIsSmall(varDsc->TypeGet()) ? TYP_INT : varDsc->TypeGet();
lvaTable[shadowVar].lvType = type;
lvaTable[shadowVar].lvType = type;

Choose a reason for hiding this comment

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

Would it make sense to have a copy method for LclVarDsc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure that i understood the suggestion. Are you implying to abstract the logic that copies varDsc fields to shadow param into a separate routine? I don't think that would be valuable, because other than gscheck logic no other place has such a need.

VarDsc has many fields but gsParamsToShadow() is selectively copying few of the fields when it creates a shadow lcl var. Are you implying to copy all fields? That would need carefully examining whether it makes sense to verbatim copy and is beyond the scope of this fix.

@CarolEidt
Copy link

What is the "other half" of #2886?

@sivarv
Copy link
Member Author

sivarv commented Feb 8, 2016

That issue has two test SIMD test failures listed in one bug. This PR is fixing the first test failure. The second test failure seems to be a different issue and still repros and under investigation. Thought it a good idea to get one fix in and hence this PR.

@CarolEidt
Copy link

@sivarv - thanks for the explanation. LGTM

@mmitche
Copy link
Member

mmitche commented Feb 9, 2016

hello world (test CI)

sivarv added a commit that referenced this pull request Feb 9, 2016
Fix to SquareRootUInt16 test failure under JitStress=1/2
@sivarv sivarv merged commit ceb0a80 into dotnet:master Feb 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

os-linux Linux OS (any supported distro)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants