-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8290892: C2: Intrinsify Reference.reachabilityFence #25315
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
base: master
Are you sure you want to change the base?
Changes from all commits
7a5ada0
8d8f2c4
ad314e0
ca2809a
1a6af8b
430622a
c73fd56
8b1c6df
0762dda
bdf1b39
e95d4eb
c9809d0
dd8a0cc
bd37e81
6981bd1
d5f94a2
267995c
01eaf64
dc37cca
68150cc
15fee72
3890119
670aab3
d80aee5
a491594
a1101cd
ed32415
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 |
|---|---|---|
|
|
@@ -611,6 +611,20 @@ void CallGenerator::do_late_inline_helper() { | |
| } | ||
|
|
||
| Compile* C = Compile::current(); | ||
|
|
||
| uint endoff = call->jvms()->endoff(); | ||
| if (C->inlining_incrementally()) { | ||
| // No reachability edges should be present when incremental inlining takes place. | ||
| // Inlining logic doesn't expect any extra edges past debug info and fails with | ||
| // an assert in SafePointNode::grow_stack. | ||
| assert(endoff == call->req(), "reachability edges not supported"); | ||
|
Comment on lines
+616
to
+620
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. Could we trip over this assert by modifying the reproducer, and add some method somewhere that gets inlined late? 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. Could we also bail out here? Or what would happen now in production if there is a RF edge? 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. We also use this area past endoff() for storing the "ex_oop" (see for example GraphKit::has_saved_ex_oop()). Are ex_oop and reachability edges mutually exclusive? 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, ex_oop and reachability edges are mutually exclusive, but there's no conflict. ex_oop is kept during parsing while reachability edges stay attached to RF nodes until loop optimizations are over (and no inlining can happen anymore). 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. @dean-long @iwanowww Ok, but probably there will at some point be a conflict. And if RF are rather rare, we will not notice so fast. Or would your stress flag catch the conflict? Is there not a way to make it clear/explicit which edges are there for what reason? 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, w/ -XX:+StressReachabilityFences it becomes very likely that a late inline call has reachability edges. |
||
| } else { | ||
| if (call->req() > endoff) { // reachability edges present | ||
| assert(OptimizeReachabilityFences, "required"); | ||
| return; // keep the original call node as the holder of reachability info | ||
| } | ||
| } | ||
|
|
||
| // Remove inlined methods from Compiler's lists. | ||
| if (call->is_macro()) { | ||
| C->remove_macro_node(call); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -541,3 +541,4 @@ macro(MaskAll) | |
| macro(AndVMask) | ||
| macro(OrVMask) | ||
| macro(XorVMask) | ||
| macro(ReachabilityFence) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,7 @@ | |
| #include "opto/output.hpp" | ||
| #include "opto/parse.hpp" | ||
| #include "opto/phaseX.hpp" | ||
| #include "opto/reachability.hpp" | ||
| #include "opto/rootnode.hpp" | ||
| #include "opto/runtime.hpp" | ||
| #include "opto/stringopts.hpp" | ||
|
|
@@ -396,6 +397,9 @@ void Compile::remove_useless_node(Node* dead) { | |
| if (dead->is_expensive()) { | ||
| remove_expensive_node(dead); | ||
| } | ||
| if (dead->is_ReachabilityFence()) { | ||
| remove_reachability_fence(dead->as_ReachabilityFence()); | ||
| } | ||
| if (dead->is_OpaqueTemplateAssertionPredicate()) { | ||
| remove_template_assertion_predicate_opaque(dead->as_OpaqueTemplateAssertionPredicate()); | ||
| } | ||
|
|
@@ -459,6 +463,7 @@ void Compile::disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_Lis | |
| // Remove useless Template Assertion Predicate opaque nodes | ||
| remove_useless_nodes(_template_assertion_predicate_opaques, useful); | ||
| remove_useless_nodes(_expensive_nodes, useful); // remove useless expensive nodes | ||
| remove_useless_nodes(_reachability_fences, useful); // remove useless node recorded for post loop opts IGVN pass | ||
| remove_useless_nodes(_for_post_loop_igvn, useful); // remove useless node recorded for post loop opts IGVN pass | ||
| remove_useless_nodes(_for_merge_stores_igvn, useful); // remove useless node recorded for merge stores IGVN pass | ||
| remove_useless_unstable_if_traps(useful); // remove useless unstable_if traps | ||
|
|
@@ -663,6 +668,7 @@ Compile::Compile(ciEnv* ci_env, ciMethod* target, int osr_bci, | |
| _parse_predicates(comp_arena(), 8, 0, nullptr), | ||
| _template_assertion_predicate_opaques(comp_arena(), 8, 0, nullptr), | ||
| _expensive_nodes(comp_arena(), 8, 0, nullptr), | ||
| _reachability_fences(comp_arena(), 8, 0, nullptr), | ||
| _for_post_loop_igvn(comp_arena(), 8, 0, nullptr), | ||
| _for_merge_stores_igvn(comp_arena(), 8, 0, nullptr), | ||
| _unstable_if_traps(comp_arena(), 8, 0, nullptr), | ||
|
|
@@ -932,6 +938,7 @@ Compile::Compile(ciEnv* ci_env, | |
| _directive(directive), | ||
| _log(ci_env->log()), | ||
| _first_failure_details(nullptr), | ||
| _reachability_fences(comp_arena(), 8, 0, nullptr), | ||
| _for_post_loop_igvn(comp_arena(), 8, 0, nullptr), | ||
| _for_merge_stores_igvn(comp_arena(), 8, 0, nullptr), | ||
| _congraph(nullptr), | ||
|
|
@@ -2512,12 +2519,21 @@ void Compile::Optimize() { | |
| return; | ||
| } | ||
|
|
||
| if (failing()) return; | ||
|
|
||
| C->clear_major_progress(); // ensure that major progress is now clear | ||
|
|
||
| process_for_post_loop_opts_igvn(igvn); | ||
|
|
||
| // Once loop optimizations are over, it is safe to get rid of all reachability fence nodes and | ||
| // migrate reachability edges to safepoints. | ||
| if (OptimizeReachabilityFences && _reachability_fences.length() > 0) { | ||
| TracePhase tp1(_t_idealLoop); | ||
| TracePhase tp2(_t_reachability); | ||
| PhaseIdealLoop::optimize(igvn, PostLoopOptsEliminateReachabilityFences); | ||
| print_method(PHASE_ELIMINATE_REACHABILITY_FENCES, 2); | ||
| if (failing()) return; | ||
| assert(_reachability_fences.length() == 0 || PreserveReachabilityFencesOnConstants, "no RF nodes allowed"); | ||
| } | ||
|
|
||
| process_for_merge_stores_igvn(igvn); | ||
|
|
||
| if (failing()) return; | ||
|
|
@@ -3973,11 +3989,29 @@ void Compile::final_graph_reshaping_walk(Node_Stack& nstack, Node* root, Final_R | |
| } | ||
| } | ||
|
|
||
| expand_reachability_fences(sfpt); | ||
|
|
||
| // Skip next transformation if compressed oops are not used. | ||
| if ((UseCompressedOops && !Matcher::gen_narrow_oop_implicit_null_checks()) || | ||
| (!UseCompressedOops && !UseCompressedClassPointers)) | ||
| return; | ||
|
|
||
| // Go over ReachabilityFence nodes to skip DecodeN nodes for referents. | ||
iwanowww marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // The sole purpose of RF node is to keep the referent oop alive and | ||
| // decoding the oop for that is not needed. | ||
| for (int i = 0; i < C->reachability_fences_count(); i++) { | ||
| ReachabilityFenceNode* rf = C->reachability_fence(i); | ||
| DecodeNNode* dn = rf->in(1)->isa_DecodeN(); | ||
| if (dn != nullptr) { | ||
| if (!dn->has_non_debug_uses() || Matcher::narrow_oop_use_complex_address()) { | ||
| rf->set_req(1, dn->in(1)); | ||
| if (dn->outcnt() == 0) { | ||
| dn->disconnect_inputs(this); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Go over safepoints nodes to skip DecodeN/DecodeNKlass nodes for debug edges. | ||
| // It could be done for an uncommon traps or any safepoints/calls | ||
| // if the DecodeN/DecodeNKlass node is referenced only in a debug info. | ||
|
|
@@ -3991,21 +4025,8 @@ void Compile::final_graph_reshaping_walk(Node_Stack& nstack, Node* root, Final_R | |
| n->as_CallStaticJava()->uncommon_trap_request() != 0); | ||
| for (int j = start; j < end; j++) { | ||
| Node* in = n->in(j); | ||
| if (in->is_DecodeNarrowPtr()) { | ||
| bool safe_to_skip = true; | ||
| if (!is_uncommon ) { | ||
| // Is it safe to skip? | ||
| for (uint i = 0; i < in->outcnt(); i++) { | ||
| Node* u = in->raw_out(i); | ||
| if (!u->is_SafePoint() || | ||
| (u->is_Call() && u->as_Call()->has_non_debug_use(n))) { | ||
| safe_to_skip = false; | ||
| } | ||
| } | ||
| } | ||
| if (safe_to_skip) { | ||
| n->set_req(j, in->in(1)); | ||
| } | ||
| if (in->is_DecodeNarrowPtr() && (is_uncommon || !in->has_non_debug_uses())) { | ||
| n->set_req(j, in->in(1)); | ||
|
Comment on lines
-3994
to
+4029
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. Can you say why you changed this code here? Is it equivalent? |
||
| if (in->outcnt() == 0) { | ||
| in->disconnect_inputs(this); | ||
| } | ||
|
|
||
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.
It could be nice if you gave some more detail here, what these flags do.