JIT: merge fgMorph into compCompile #2110
Conversation
This gives us a single method that controls most of the jit's phase behavior. Largely a manual inline, though I updated some comments and changed one assert. Follow up from dotnet#1309; also see dotnet#2109.
|
@dotnet/jit-contrib PTAL |
CarolEidt
left a comment
CarolEidt
left a comment
There was a problem hiding this comment.
LGTM, with minor comment suggestions
src/coreclr/src/jit/compiler.cpp
Outdated
| } | ||
|
|
||
| assert(!compDonotInline()); | ||
| // By this point we are only jitting the root method (and not an inlinee) |
There was a problem hiding this comment.
Minor: I might reword this as "By this point we have completed inlining into the root method, so going forward we won't encounter any inlinees" or something like that.
There was a problem hiding this comment.
At this point we haven't started inlining into the root method.
We run a few of the initial phases on inlinees. So inlinees and root methods both call compCompile. This comment is trying to say that "at this point in the phase list, all the inlinee phases have been run, and inlinee compiles have exited, so we should only get this far if we are jitting the root method."
If that quoted text seems better, let me know, and I'll update...
There was a problem hiding this comment.
Yes, that would be great - and thanks for clarifying!
src/coreclr/src/jit/compiler.cpp
Outdated
| fgDomsComputed = false; | ||
|
|
||
| /* Create LSRA before Lowering, this way Lowering can initialize the TreeNode Map */ | ||
| // Create LSRA before Lowering, this way Lowering can initialize the TreeNode Map |
There was a problem hiding this comment.
Nit: since you're here, the TreeNode map is gone; I would reword this as something like "Create LinearScan before Lowering, so that Lowering can call LinearScan methods for determining whether locals are register candidates and (for xarch) whether a node is a containable memory op.
src/coreclr/src/jit/compiler.cpp
Outdated
| NewBasicBlockEpoch(); | ||
|
|
||
| /* Massage the trees so that we can generate code out of them */ | ||
| // Start of the code that used to make up fgMorph |
There was a problem hiding this comment.
I'd avoid mentioning what used to be, and instead encompass the comment below, to say something like: "Start of the code that is broadly called morphing, and includes global morph, as well as other phases that massage the trees so that we can generate code out of them."
sandreenko
added
the
area-CodeGen-coreclr
|
Failures almost certainly unrelated. |
BruceForstall
left a comment
BruceForstall
left a comment
There was a problem hiding this comment.
Seems like we could simplify this even further as a further change by sinking checks and #ifdefs into phase / function calls, extracting functions for things like compGcChecks, etc., so the function ends up being almost entirely function calls to phase-like functions and EndPhase calls.
| // | ||
| // Arguments: | ||
| // methodCodePtr [OUT] - address of generated code | ||
| // methodCodeSize [OUT] - size of the generated code (hot + cold setions) |
There was a problem hiding this comment.
Thanks; will fix this in subsequent change.
That's where I am headed (in gradual steps), see #2109... |
ghost
locked as resolved and limited conversation to collaborators
4 participants
This gives us a single method that controls most of the jit's phase behavior.
Largely a manual inline, though I updated some comments and changed one assert.
Follow up from #1309; also see #2109.