Convert more jit phases to opt into common post phase checks#74041
Convert more jit phases to opt into common post phase checks#74041AndyAyersMS merged 13 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThere are 20-odd phases that haven't yet been converted to running the common post phase checks. This converts the first half of them. Note this is a bit of a lie. We actually don't run most checks until morph, but a future change will work on enabling checks throughout the phase list. I also did a bit of cleanup here and there; most notably we no longer needed Contributes to #2109.
|
|
@dotnet/jit-contrib PTAL |
src/coreclr/jit/fgopt.cpp
Outdated
There was a problem hiding this comment.
It doesn't look like this change factors into the decision about returning MODIFIED_NOTHING below.
Also, presumably doing this before this phase is equivalent to doing it after, where it was being done before?
There was a problem hiding this comment.
I haven't been trying to track modifications that we don't check and/or dump anywhere.
src/coreclr/jit/fgopt.cpp
Outdated
There was a problem hiding this comment.
(not related to this line)
Interesting that we don't consider removing BBF_REMOVED blocks as modified but I guess that makes sense.
src/coreclr/jit/fgprofile.cpp
Outdated
There was a problem hiding this comment.
nit
| // return weight [out] - sum of weights for all return and throw blocks | |
| // returnWeight [out] - sum of weights for all return and throw blocks |
src/coreclr/jit/flowgraph.cpp
Outdated
There was a problem hiding this comment.
Instead of tracking madeChanges you could just use:
| return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; | |
| return (compHndBBtabCount > 0) ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; |
src/coreclr/jit/phase.cpp
Outdated
There was a problem hiding this comment.
Do you want to un-comment this?
There was a problem hiding this comment.
I had it that way at one point, but it caused loop table validity asserts.
Will revisit as part of round 2 when I try converting the rest of the phases.
|
SPMI failures are likely from #64497, will merge up next time I push. |
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
All the jit stress failures are arm/arm64 infra issues. SPMI are timeouts / and or rerun failures because of pre-existing logs. Going to roll the dice here and say there's no reason to try and get all those green. |
There are 20-odd phases that haven't yet been converted to running the common post phase checks. This converts the first half of them.
Note this is a bit of a lie. We actually don't run most checks until morph, but a future change will work on enabling checks throughout the phase list.
I also did a bit of cleanup here and there; most notably we no longer needed
fgResetImplicitByRefRefCountas it was just setting the counts the values they already had.Contributes to #2109.