-
Notifications
You must be signed in to change notification settings - Fork 113
8335256: [lworld] C2: Remove larval InlineTypeNode #1447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ae3ed36
d9e233e
d91fcf9
3be7b97
9cedae4
bdc01f4
4b16f6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2843,16 +2843,39 @@ void Compile::Optimize() { | |
|
||
if (failing()) return; | ||
|
||
{ | ||
// Eliminate some macro nodes before EA to reduce analysis pressure | ||
PhaseMacroExpand mexp(igvn); | ||
mexp.eliminate_macro_nodes(); | ||
if (failing()) { | ||
return; | ||
} | ||
igvn.set_delay_transform(false); | ||
print_method(PHASE_ITER_GVN_AFTER_ELIMINATION, 2); | ||
} | ||
|
||
// Perform escape analysis | ||
if (do_escape_analysis() && ConnectionGraph::has_candidates(this)) { | ||
if (has_loops()) { | ||
// Cleanup graph (remove dead nodes). | ||
TracePhase tp(_t_idealLoop); | ||
PhaseIdealLoop::optimize(igvn, LoopOptsMaxUnroll); | ||
if (failing()) return; | ||
if (failing()) { | ||
return; | ||
} | ||
print_method(PHASE_PHASEIDEAL_BEFORE_EA, 2); | ||
|
||
// Eliminate some macro nodes before EA to reduce analysis pressure | ||
PhaseMacroExpand mexp(igvn); | ||
mexp.eliminate_macro_nodes(); | ||
if (failing()) { | ||
return; | ||
} | ||
igvn.set_delay_transform(false); | ||
print_method(PHASE_ITER_GVN_AFTER_ELIMINATION, 2); | ||
} | ||
|
||
bool progress; | ||
print_method(PHASE_PHASEIDEAL_BEFORE_EA, 2); | ||
do { | ||
ConnectionGraph::do_analysis(this, &igvn); | ||
|
||
|
@@ -2870,12 +2893,10 @@ void Compile::Optimize() { | |
TracePhase tp(_t_macroEliminate); | ||
PhaseMacroExpand mexp(igvn); | ||
mexp.eliminate_macro_nodes(); | ||
if (failing()) return; | ||
|
||
if (failing()) { | ||
return; | ||
} | ||
igvn.set_delay_transform(false); | ||
igvn.optimize(); | ||
if (failing()) return; | ||
|
||
print_method(PHASE_ITER_GVN_AFTER_ELIMINATION, 2); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, you added it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think it is cleaner that way instead of trailing each |
||
} | ||
|
||
|
@@ -2976,8 +2997,14 @@ void Compile::Optimize() { | |
|
||
{ | ||
TracePhase tp(_t_macroExpand); | ||
PhaseMacroExpand mex(igvn); | ||
// Last attempt to eliminate macro nodes. | ||
mex.eliminate_macro_nodes(); | ||
if (failing()) { | ||
return; | ||
} | ||
|
||
print_method(PHASE_BEFORE_MACRO_EXPANSION, 3); | ||
PhaseMacroExpand mex(igvn); | ||
if (mex.expand_macro_nodes()) { | ||
assert(failing(), "must bail out w/ explicit message"); | ||
return; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we should consider doing in mainline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what effect it will have on mainline. For this patch, without this,
TestNullableInlineTypes::test85
tries to enter EA with 10k nodes, which it bails out. Trying to eliminate all trivially removable allocations reduces the number to 2k, allows EA to complete successfully. I think it may be more significant on Valhalla because in general we seem to emit a lot more nodes with value objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know what you think about this, perhaps we need to measure the effects on mainline first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I'd say we put it into Valhalla only for now. We can still consider upstreaming it later.