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

Correctly sequence fgMorphModToSubMulDiv#8642

Merged
CarolEidt merged 1 commit intodotnet:masterfrom
CarolEidt:Fix359736
Dec 15, 2016
Merged

Correctly sequence fgMorphModToSubMulDiv#8642
CarolEidt merged 1 commit intodotnet:masterfrom
CarolEidt:Fix359736

Conversation

@CarolEidt
Copy link
Copy Markdown

This method was creating a temp, but the final result was a GT_SUB with
a use of the temp as its op1, and it was not setting GTF_REVERSE_OPS.
This led to a liveness assert in LSRA.

This method was creating a temp, but the final result was a GT_SUB with
a use of the temp as its op1, and it was not setting GTF_REVERSE_OPS.
This led to a liveness assert in LSRA.
@mikedn
Copy link
Copy Markdown

mikedn commented Dec 14, 2016

Oops, I take it this was indirectly caused by my recent change that moved magic division to lowering. But how come this worked for ARM64? Lack of tests?

@CarolEidt
Copy link
Copy Markdown
Author

@mikedn - Yes, the specific scenario needed to reproduce this wasn't covered by existing tests. But at least it was reproducible with a very small test case :-)

@mikedn
Copy link
Copy Markdown

mikedn commented Dec 14, 2016

Hmm, I need to take a closer look at this. It seems to me that even with this fix something doesn't work right, if we do arg % 0 we end up with arg, the division by 0 is nowhere to be seen and no exception is thrown. There's actually a bug in my code, it should not call fgMorphModToSubMulDiv if the divisor is 0. But then the ARM64 specific morph code appears to be calling fgMorphModToSubMulDiv unconditionally so presumably it should handle division by 0 correctly.

@mikedn
Copy link
Copy Markdown

mikedn commented Dec 15, 2016

I opened a separate issue for the problem I'm seeing, it appears to be completely unrelated to the bug you are fixing here - #8648.

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet/jit-contrib PTAL. This fixes bug 359736

@sivarv
Copy link
Copy Markdown
Member

sivarv commented Dec 15, 2016

:shipit:

@briansull
Copy link
Copy Markdown

LGTM

@CarolEidt CarolEidt merged commit 3da9854 into dotnet:master Dec 15, 2016
@CarolEidt CarolEidt deleted the Fix359736 branch December 15, 2016 19:02
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Correctly sequence fgMorphModToSubMulDiv

Commit migrated from dotnet/coreclr@3da9854
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.

6 participants