JIT: rework phase objects so we can use them more widely#31808
JIT: rework phase objects so we can use them more widely#31808AndyAyersMS merged 2 commits intodotnet:masterfrom
Conversation
Next part of dotnet#2109. Add support for phases that are simply compiler methods or lambdas. These are not used yet -- the plan is to introduce new phases gradually, cleaning up redundant checking and dumping along the way. This will happen in subequent changes. Remove a bit of now-redundant post-phase dumping and checking from lower. Add the active phase to the assertion text, so we have some context.
|
cc @dotnet/jit-contrib No diffs; jit dump is a bit smaller as there were basically two IR dumps at the end of lower. |
CarolEidt
left a comment
There was a problem hiding this comment.
LGTM - I really like the direction this is going! I'm not sure I would have added the method/lambda version until it's used, but no harm I guess.
|
|
||
| // End of the morphing phases | ||
| // | ||
| // We can now enable all phase checking |
There was a problem hiding this comment.
Nice! Might be worth a comment explaining why this can't be done earlier.
There was a problem hiding this comment.
This should be temporary (see below).
| LogEnv* env = JitTls::GetLogEnv(); | ||
| const int BUFF_SIZE = 8192; | ||
| char* buff = (char*)alloca(BUFF_SIZE); | ||
| const char* phaseName = "unknown phase"; |
There was a problem hiding this comment.
Adding the phase to the assert is great!
|
I have another change that phasifies everything (and uses the new phase forms introduced here) but it currently introduces a lot of redundant dumping and checking. I do not want to make checked builds slower or use more memory or make jit dump files larger. So I have to work towards this bit by bit. The next few steps are:
|
|
Maybe you could start by having a phase opt-in to post-phase checks/dumps that it already does, remove those manual steps, and have the phase infrastructure do them. |
|
I had originally thought to strongly couple phases to post-phase checks, but didn't like how this came out, it was a lot of extra detail. Current proposal is that the set of active post-phase checks will be part of the ambient compiler state, so as we progress through the phase list we will add / remove active checks. We can do this either in between or during phases. On top of this, I may add a way for a phase to request "additional" checks, if there are phases that warrant extra scrutiny and the extra checks are not specific to just one phase. |
| # add the install targets | ||
| install_clr(TARGETS ${jitName}) | ||
| endfunction() | ||
|
|
There was a problem hiding this comment.
hmmm... you don't like whitespace?
There was a problem hiding this comment.
Didn't mean to do that. Let me undo that part.
Next part of #2109.
Add support for phases that are simply compiler methods or lambdas. These are
not used yet -- the plan is to introduce new phases gradually, cleaning up
redundant checking and dumping along the way. This will happen in subequent
changes.
Remove a bit of now-redundant post-phase dumping and checking from lower.
Add the active phase to the assertion text, so we have some context.