Fold null checks against known non-null values #109164
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@MihuBot -dependsOn 108579 |
src/coreclr/jit/gentree.cpp
Outdated
|
|
||
| return NewMorphedIntConNode(compareResult); | ||
| GenTree* newTree = gtNewIconNode(compareResult); | ||
| if (wrapEffects) |
There was a problem hiding this comment.
just wrap unconditionally, gtWrapWithSideEffects won't create a COMMA if there are no side-effects
There was a problem hiding this comment.
AFAIR we can't use the op if gtTryRemoveBoxUpstreamEffects removes the box, that's why I did the check.
There was a problem hiding this comment.
not sure I understand, if it removes - does it leave a tree with a side-effect?
There was a problem hiding this comment.
not sure I understand, if it removes - does it leave a tree with a side-effect?
As far as @SingleAccretion explained it to me on Discord, using the tree when gtTryRemoveBoxUpstreamEffects succeedes is nonsensical since the original tree is invalid due to the earlier box not existing anymore.
src/coreclr/jit/gentree.cpp
Outdated
| GenTree* boxSourceTree = gtTryRemoveBoxUpstreamEffects(op); | ||
| bool didOptimize = (boxSourceTree != nullptr); | ||
| // See if we can optimize away the box and related statements. | ||
| wrapEffects = (gtTryRemoveBoxUpstreamEffects(op) == nullptr); |
There was a problem hiding this comment.
previous logic used to give up if box can't be removed, is it expected that the new one always folds?
There was a problem hiding this comment.
btw, does it all handle boxed nullables (when boxed value is null reference)?
There was a problem hiding this comment.
is it expected that the new one always folds?
Yeah, my reasoning here was that there's no reason to avoid folding if we can't remove.
does it all handle boxed nullables (when boxed value is null reference)?
Not sure, I'd assume those wouldn't pass IsBoxedValue, otherwise I'd think the previous code would be broken.
EDIT: They wouldn't, IsBoxedValue checks GTF_BOX_VALUE which guarantees it's not null.
|
@MihuBot -dependsOn 109715 |
Found in #108579.