-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Do not generate TYP_STRUCT LCL_FLD nodes
#66251
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
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsToday, the morpher can fold These will be a significant impediment to actual, proper This change fixed that by introducing the folding that the creation of these location nodes achieves at the address level, where we can choose the type of the location node arbitrarily and freely. Along the way, We are expecting some positive diffs here, from the We are also expecting one small regression due to call retyping of reg-sized Blocked on #66247.
|
f5a918a to
a6cae9a
Compare
This is also a correctness fix. Missing adding zero-offset field sequences is fatal.
Types of location nodes do not matter, the underlying locations do.
a6cae9a to
acf22a0
Compare
|
@dotnet/jit-contrib |
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.
Couple of comments.
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.
LGTM
|
The tasks seems to be complete but not sure why it is not reflecting in GH. I will go ahead and merge this. |
This reverts commit 2453f16.
This reverts commit 2453f16.
Today, the morpher can fold
IND(struct ADDR(LCL))intoLCL_FLDTYP_STRUCTlayout-less nodes, these end up being locations, always (well -- there is one case where that's not true, such a node can end up under a return of a small struct later to be retyped by lowering, but this is rare).These will be a significant impediment to actual, proper
TYP_STRUCTlocal field nodes, as it would mean handling the case, everywhere in code, where the layout would be missing, that we don't want to allow in the first place.This change fixed that by introducing the folding that the creation of these location nodes achieves at the address level, where we can choose the type of the location node arbitrarily and freely.
Along the way,
IsLocalAddrExprwas fixed to not forget to look at zero-offset sequences attached toADDRs, to avoid regressions.We are expecting some positive diffs here, from the
IsLocalAddrExprchange (enabling more precise VNs) and the folding change itself (the old code path would, ever so rarely, miss out on some things).We are also expecting one small regression due to call retyping of reg-sized
IND struct(ADDR(LCL))nodes losing the field sequence. I did not fix it because a) it was small, and rare, b) I do not want to permeate the zero-offset code any more than is strictly required.Diffs.