Allow gtFoldExprSpecial to handle side effects #103382
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
tannergooding
force-pushed
the
fold-int
branch
3 times, most recently
from
b449a29
to
960870a
Compare
|
CC. @dotnet/jit-contrib for review |
| GenTree* potentialCall = tree; | ||
|
|
||
| if (potentialCall->OperIs(GT_RET_EXPR)) | ||
| { | ||
| // We need to preserve return expressions where the underlying call | ||
| // has side effects. Otherwise early folding can result in us dropping | ||
| // the call. | ||
| potentialCall = potentialCall->AsRetExpr()->gtInlineCandidate; | ||
| } |
There was a problem hiding this comment.
To comment on this fix a bit more...
gtNodeHasSideEffects is trying to see if it really has side effects or is just marked such because some child node has side effects
It's mainly used as part of gtExtractSideEffList which is trying to build a list of all the real side effects
throwing away anything that isn't itself directly side effecting
i.e. since ADD(call, cns) would be marked as CALL due to propagating side effect flags up through the parent and therefore we want to just save the call in the produced comma, not the add or cns
so it was just missing the fact that GT_RET_EXPR is technically a call itself, just a placeholder until the latter IR transforms take place
which is relevant since the inliner, if it fails, actually removes the block containing the call assuming that GT_RET_EXPR will replace itself with the call
but, we'd have dropped the GT_RET_EXPR as non side effecting if it happened too early (such as happens for gtFoldExprSpecial since it happens as part of normal IL importation and evaluation)
This just updates, as per the title,
gtFoldExprSpecialto allow handling of side effects rather than skipping them.As part of that, it turns out we weren't tracking
GT_RET_EXPRas side effecting, so ifgtNodeHasSideEffectswas used before the inlining pass and them getting replaced with the result or the original call, then we could have dropped them thinking they were non side effecting. So I ensured we check if the underlying call was side effecting and report based on that instead.Not many diffs, but a small TP improvement.