Update full-struct references to promoted IBR args#11883
Conversation
Update fgMorphImplicitByRefArgs to rewrite references to struct-promoted implicit-by-reference parameters as references to the new struct temps created in fgRetypeImplicitByRefArgs; fgMorphStructField isn't updating these because it's only interested in field references, and runs upstream of fgRetypeImplicitByRefArgs where the full struct temp is created, anyway. Invert the sense of lvPromoted for implicit byref args in the interim between fgRetypeImplicitByRefArgs and fgMarkDemotedImplicitByRefArgs, since now fgMarkDemotedImplicitByRefArgs needs to update both and would look horribly backwards the other way around. Fixes #11814.
|
@briansull PTAL. I've verified that this fixes the repro from dotnet/corefx#20025 in addittion to #11814, though it doesn't fix #11831 |
JosephTremoulet
force-pushed
the
PromotedStructRefs
branch
from
ed22d7b to
0057a2f
Compare
CarolEidt
left a comment
CarolEidt
left a comment
There was a problem hiding this comment.
LGTM modulo some comments and a question
| // have these fields. | ||
| varDsc->lvFieldCnt = 0; | ||
|
|
||
| // Hijack lvPromoted to communicate to fgMorphImplicitByRefArgs |
There was a problem hiding this comment.
Please add comments on these fields in compiler.h, even though these are transient.
| { | ||
| if (varDsc->lvPromoted) | ||
| { | ||
| // The parameter is simply a pointer now, so clear lvPromoted. |
There was a problem hiding this comment.
This is something that could be confusing if viewed in isolation. You might also mention here that this field has been used temporarily to communicate between fgRetypeImplicitByRefArgs() and this method.
| // Here we have an arg that was simply never promoted, so make sure it doesn't | ||
| // have nonzero lvFieldLclStart, since that would confuse fgMorphImplicitByRefArgs | ||
| // and fgMarkDemotedImplicitByRefArgs. | ||
| assert(varDsc->lvFieldLclStart == 0); |
There was a problem hiding this comment.
This is really applicable to the previous checkin, but I'm wondering why you're not using BAD_VAR_NUM for these, since 0 is a valid varNum (not that it could ever be a valid lvFieldLclStart)
There was a problem hiding this comment.
That's just the way it gets initialized when the node is created, the ones with 0 for lvFieldLclStart are the ones I'm not touching at all.
A number of fields on the LclVarDsc get hijacked during the multi-stage rewrite of implicit-by-reference parameters; add comments at the fields' declarations as well as the hijacking uses.
|
Comments updated. |
5 participants
Update fgMorphImplicitByRefArgs to rewrite references to struct-promoted
implicit-by-reference parameters as references to the new struct temps
created in fgRetypeImplicitByRefArgs; fgMorphStructField isn't updating
these because it's only interested in field references, and runs upstream
of fgRetypeImplicitByRefArgs where the full struct temp is created,
anyway.
Invert the sense of lvPromoted for implicit byref args in the interim
between fgRetypeImplicitByRefArgs and fgMarkDemotedImplicitByRefArgs,
since now fgMarkDemotedImplicitByRefArgs needs to update both and would
look horribly backwards the other way around.
Fixes #11814.