-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update the assert to correctly check if a tree with value has non-zero destination registers #81412
Conversation
@dotnet/jit-contrib |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsIn #77078, we based an indir to Fixes: #81356
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the node in lowering instead of creating a NOP? All other places in lowering do BlockRange().Remove(node)
instead of bashing to a NOP.
It's also odd to me that GenTree::IsUnusedValue()
returns true for the NOP here, given that it isn't even a value.
Ah, my impression was that this would have got removed with liveness update after lowering, but didn't realize I could just delete it. The fix in lsra is still needed though because that assert was wrong.
I am not sure about it, but we do call |
Thanks @kunalspathak for this fix. |
This reverts commit 32a8c9c.
In #77078, we based an indir to
NOP
and in MinOpts, since we do not perform liveness, it stays until LSRA. In LSRA, during building the nodes, we should be checking if a tree has a value, the number of destination registers should be non-zero. Instead, we were checking if tree has unused value, it should have non-zero destination registers, which was not making sense. Updated the assert.Fixes: #81356