Small TP improvement in assertprop #113360
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet/jit-contrib PTAL, simple change with diffs (size & TP). The vast majority of all assertions are "lclvar != 0" (both local and global assert prop) created by GT_IND node (and GT_STOREIND), ignoring GTF_EXCEPT alone gives nice results, but there should be done something smarter to avoid creating redundant assertions. |
|
Ah, actually, I should probably keep the "is address exposed" check for local prop, it might not check that |
|
@jakobbotsch @AndyAyersMS does this look reasonable? I plan to make LCLVAR local-prop only, remove Another thing to experiment is to speed up "do we already have this assertion?" currently it's O(n) loop and it shows up in a profiler. |
src/coreclr/jit/assertionprop.cpp
Outdated
| @@ -2370,7 +2337,11 @@ void Compiler::optAssertionGen(GenTree* tree) | |||
| case GT_ARR_LENGTH: | |||
| case GT_MDARR_LENGTH: | |||
| case GT_MDARR_LOWER_BOUND: | |||
| assertionInfo = optCreateAssertion(tree->GetIndirOrArrMetaDataAddr(), nullptr, OAK_NOT_EQUAL); | |||
| // These indirs (esp. GT_IND and GT_STOREIND) are the most popular sources of assertions. | |||
| if ((tree->gtFlags & GTF_EXCEPT) != 0) | |||
There was a problem hiding this comment.
Seems it might make more sense to use IndirMayFault() here, since this check conflates the exceptions thrown by operands with the indir itself.
There was a problem hiding this comment.
Good point, but looks like IndirMayFault may not be cheap. In this case it's just a heuristic to avoid creating too many assertions (and we do create too many)
Would appreciate a green one 🙂 |
Oops, sorry about that |
Diffs - they come from the GTF_EXCEPT change. the rest of the changes are just clean up. E.g. I removed various
ValueNumStore::NoVNchecks becauseoptAddAssertionis already smart enough to ignore assertions with such vns.