Skip to content

Commit 9cca4f7

Browse files
author
Vladimir Ivanov
committed
8358751: C2: Recursive inlining check for compiled lambda forms is broken
Reviewed-by: dlong, roland
1 parent 9f4d5b2 commit 9cca4f7

File tree

4 files changed

+75
-34
lines changed

4 files changed

+75
-34
lines changed

src/hotspot/share/opto/bytecodeInfo.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ InlineTree::InlineTree(Compile* c,
6060
// Keep a private copy of the caller_jvms:
6161
_caller_jvms = new (C) JVMState(caller_jvms->method(), caller_tree->caller_jvms());
6262
_caller_jvms->set_bci(caller_jvms->bci());
63+
_caller_jvms->set_receiver_info(caller_jvms->receiver_info());
6364
assert(!caller_jvms->should_reexecute(), "there should be no reexecute bytecode with inlining");
6465
assert(_caller_jvms->same_calls_as(caller_jvms), "consistent JVMS");
6566
}
@@ -437,24 +438,26 @@ bool InlineTree::try_to_inline(ciMethod* callee_method, ciMethod* caller_method,
437438

438439
// detect direct and indirect recursive inlining
439440
{
440-
// count the current method and the callee
441441
const bool is_compiled_lambda_form = callee_method->is_compiled_lambda_form();
442-
int inline_level = 0;
443-
if (!is_compiled_lambda_form) {
444-
if (method() == callee_method) {
445-
inline_level++;
446-
}
442+
const bool is_method_handle_invoker = is_compiled_lambda_form && !jvms->method()->is_compiled_lambda_form();
443+
444+
ciInstance* lform_callee_recv = nullptr;
445+
if (is_compiled_lambda_form && !is_method_handle_invoker) { // MH invokers don't have a receiver
446+
lform_callee_recv = jvms->compute_receiver_info(callee_method);
447447
}
448-
// count callers of current method and callee
449-
Node* callee_argument0 = is_compiled_lambda_form ? jvms->map()->argument(jvms, 0)->uncast() : nullptr;
450-
for (JVMState* j = jvms->caller(); j != nullptr && j->has_method(); j = j->caller()) {
448+
449+
int inline_level = 0;
450+
for (JVMState* j = jvms; j != nullptr && j->has_method(); j = j->caller()) {
451451
if (j->method() == callee_method) {
452-
if (is_compiled_lambda_form) {
453-
// Since compiled lambda forms are heavily reused we allow recursive inlining. If it is truly
454-
// a recursion (using the same "receiver") we limit inlining otherwise we can easily blow the
455-
// compiler stack.
456-
Node* caller_argument0 = j->map()->argument(j, 0)->uncast();
457-
if (caller_argument0 == callee_argument0) {
452+
// Since compiled lambda forms are heavily reused we allow recursive inlining. If it is truly
453+
// a recursion (using the same "receiver") we limit inlining otherwise we can easily blow the
454+
// compiler stack.
455+
if (lform_callee_recv != nullptr) {
456+
ciInstance* lform_caller_recv = j->receiver_info();
457+
assert(lform_caller_recv != nullptr || j->depth() == 1 ||
458+
!j->caller()->method()->is_compiled_lambda_form(), // MH invoker
459+
"missing receiver info");
460+
if (lform_caller_recv == lform_callee_recv || lform_caller_recv == nullptr) {
458461
inline_level++;
459462
}
460463
} else {

src/hotspot/share/opto/callnode.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ uint TailJumpNode::match_edge(uint idx) const {
262262

263263
//=============================================================================
264264
JVMState::JVMState(ciMethod* method, JVMState* caller) :
265-
_method(method) {
265+
_method(method),
266+
_receiver_info(nullptr) {
266267
assert(method != nullptr, "must be valid call site");
267268
_bci = InvocationEntryBci;
268269
_reexecute = Reexecute_Undefined;
@@ -278,7 +279,8 @@ JVMState::JVMState(ciMethod* method, JVMState* caller) :
278279
_sp = 0;
279280
}
280281
JVMState::JVMState(int stack_size) :
281-
_method(nullptr) {
282+
_method(nullptr),
283+
_receiver_info(nullptr) {
282284
_bci = InvocationEntryBci;
283285
_reexecute = Reexecute_Undefined;
284286
DEBUG_ONLY(_map = (SafePointNode*)-1);
@@ -613,6 +615,7 @@ JVMState* JVMState::clone_shallow(Compile* C) const {
613615
n->set_endoff(_endoff);
614616
n->set_sp(_sp);
615617
n->set_map(_map);
618+
n->set_receiver_info(_receiver_info);
616619
return n;
617620
}
618621

@@ -687,6 +690,20 @@ int JVMState::interpreter_frame_size() const {
687690
return size + Deoptimization::last_frame_adjust(0, callee_locals) * BytesPerWord;
688691
}
689692

693+
// Compute receiver info for a compiled lambda form at call site.
694+
ciInstance* JVMState::compute_receiver_info(ciMethod* callee) const {
695+
assert(callee != nullptr && callee->is_compiled_lambda_form(), "");
696+
if (has_method() && method()->is_compiled_lambda_form()) { // callee is not a MH invoker
697+
Node* recv = map()->argument(this, 0);
698+
assert(recv != nullptr, "");
699+
const TypeOopPtr* recv_toop = recv->bottom_type()->isa_oopptr();
700+
if (recv_toop != nullptr && recv_toop->const_oop() != nullptr) {
701+
return recv_toop->const_oop()->as_instance();
702+
}
703+
}
704+
return nullptr;
705+
}
706+
690707
//=============================================================================
691708
bool CallNode::cmp( const Node &n ) const
692709
{ return _tf == ((CallNode&)n)._tf && _jvms == ((CallNode&)n)._jvms; }
@@ -1364,7 +1381,7 @@ void CallLeafNode::dump_spec(outputStream *st) const {
13641381

13651382
//=============================================================================
13661383

1367-
void SafePointNode::set_local(JVMState* jvms, uint idx, Node *c) {
1384+
void SafePointNode::set_local(const JVMState* jvms, uint idx, Node *c) {
13681385
assert(verify_jvms(jvms), "jvms must match");
13691386
int loc = jvms->locoff() + idx;
13701387
if (in(loc)->is_top() && idx > 0 && !c->is_top() ) {

src/hotspot/share/opto/callnode.hpp

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ class JVMState : public ResourceObj {
217217
int _bci; // Byte Code Index of this JVM point
218218
ReexecuteState _reexecute; // Whether this bytecode need to be re-executed
219219
ciMethod* _method; // Method Pointer
220+
ciInstance* _receiver_info; // Constant receiver instance for compiled lambda forms
220221
SafePointNode* _map; // Map node associated with this scope
221222
public:
222223
friend class Compile;
@@ -259,6 +260,7 @@ class JVMState : public ResourceObj {
259260
bool is_reexecute_undefined() const { return _reexecute==Reexecute_Undefined; }
260261
bool has_method() const { return _method != nullptr; }
261262
ciMethod* method() const { assert(has_method(), ""); return _method; }
263+
ciInstance* receiver_info() const { assert(has_method(), ""); return _receiver_info; }
262264
JVMState* caller() const { return _caller; }
263265
SafePointNode* map() const { return _map; }
264266
uint depth() const { return _depth; }
@@ -304,13 +306,15 @@ class JVMState : public ResourceObj {
304306
// _reexecute is initialized to "undefined" for a new bci
305307
void set_bci(int bci) {if(_bci != bci)_reexecute=Reexecute_Undefined; _bci = bci; }
306308
void set_should_reexecute(bool reexec) {_reexecute = reexec ? Reexecute_True : Reexecute_False;}
309+
void set_receiver_info(ciInstance* recv) { assert(has_method() || recv == nullptr, ""); _receiver_info = recv; }
307310

308311
// Miscellaneous utility functions
309312
JVMState* clone_deep(Compile* C) const; // recursively clones caller chain
310313
JVMState* clone_shallow(Compile* C) const; // retains uncloned caller
311314
void set_map_deep(SafePointNode *map);// reset map for all callers
312315
void adapt_position(int delta); // Adapt offsets in in-array after adding an edge.
313316
int interpreter_frame_size() const;
317+
ciInstance* compute_receiver_info(ciMethod* callee) const;
314318

315319
#ifndef PRODUCT
316320
void print_method_with_lineno(outputStream* st, bool show_name) const;
@@ -374,7 +378,7 @@ class SafePointNode : public MultiNode {
374378
}
375379

376380
private:
377-
void verify_input(JVMState* jvms, uint idx) const {
381+
void verify_input(const JVMState* jvms, uint idx) const {
378382
assert(verify_jvms(jvms), "jvms must match");
379383
Node* n = in(idx);
380384
assert((!n->bottom_type()->isa_long() && !n->bottom_type()->isa_double()) ||
@@ -383,34 +387,44 @@ class SafePointNode : public MultiNode {
383387

384388
public:
385389
// Functionality from old debug nodes which has changed
386-
Node *local(JVMState* jvms, uint idx) const {
387-
verify_input(jvms, jvms->locoff() + idx);
388-
return in(jvms->locoff() + idx);
390+
Node* local(const JVMState* jvms, uint idx) const {
391+
uint loc_idx = jvms->locoff() + idx;
392+
assert(jvms->is_loc(loc_idx), "not a local slot");
393+
verify_input(jvms, loc_idx);
394+
return in(loc_idx);
389395
}
390-
Node *stack(JVMState* jvms, uint idx) const {
391-
verify_input(jvms, jvms->stkoff() + idx);
392-
return in(jvms->stkoff() + idx);
396+
Node* stack(const JVMState* jvms, uint idx) const {
397+
uint stk_idx = jvms->stkoff() + idx;
398+
assert(jvms->is_stk(stk_idx), "not a stack slot");
399+
verify_input(jvms, stk_idx);
400+
return in(stk_idx);
393401
}
394-
Node *argument(JVMState* jvms, uint idx) const {
395-
verify_input(jvms, jvms->argoff() + idx);
402+
Node* argument(const JVMState* jvms, uint idx) const {
403+
uint arg_idx = jvms->argoff() + idx;
404+
assert(jvms->is_stk(arg_idx), "not an argument slot");
405+
verify_input(jvms, arg_idx);
396406
return in(jvms->argoff() + idx);
397407
}
398-
Node *monitor_box(JVMState* jvms, uint idx) const {
408+
Node* monitor_box(const JVMState* jvms, uint idx) const {
399409
assert(verify_jvms(jvms), "jvms must match");
400-
return in(jvms->monitor_box_offset(idx));
410+
uint mon_box_idx = jvms->monitor_box_offset(idx);
411+
assert(jvms->is_monitor_box(mon_box_idx), "not a monitor box offset");
412+
return in(mon_box_idx);
401413
}
402-
Node *monitor_obj(JVMState* jvms, uint idx) const {
414+
Node* monitor_obj(const JVMState* jvms, uint idx) const {
403415
assert(verify_jvms(jvms), "jvms must match");
404-
return in(jvms->monitor_obj_offset(idx));
416+
uint mon_obj_idx = jvms->monitor_obj_offset(idx);
417+
assert(jvms->is_mon(mon_obj_idx) && !jvms->is_monitor_box(mon_obj_idx), "not a monitor obj offset");
418+
return in(mon_obj_idx);
405419
}
406420

407-
void set_local(JVMState* jvms, uint idx, Node *c);
421+
void set_local(const JVMState* jvms, uint idx, Node *c);
408422

409-
void set_stack(JVMState* jvms, uint idx, Node *c) {
423+
void set_stack(const JVMState* jvms, uint idx, Node *c) {
410424
assert(verify_jvms(jvms), "jvms must match");
411425
set_req(jvms->stkoff() + idx, c);
412426
}
413-
void set_argument(JVMState* jvms, uint idx, Node *c) {
427+
void set_argument(const JVMState* jvms, uint idx, Node *c) {
414428
assert(verify_jvms(jvms), "jvms must match");
415429
set_req(jvms->argoff() + idx, c);
416430
}

src/hotspot/share/opto/parse1.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,13 @@ SafePointNode* Parse::create_entry_map() {
11491149
// Create an initial safepoint to hold JVM state during parsing
11501150
JVMState* jvms = new (C) JVMState(method(), _caller->has_method() ? _caller : nullptr);
11511151
set_map(new SafePointNode(len, jvms));
1152+
1153+
// Capture receiver info for compiled lambda forms.
1154+
if (method()->is_compiled_lambda_form()) {
1155+
ciInstance* recv_info = _caller->compute_receiver_info(method());
1156+
jvms->set_receiver_info(recv_info);
1157+
}
1158+
11521159
jvms->set_map(map());
11531160
record_for_igvn(map());
11541161
assert(jvms->endoff() == len, "correct jvms sizing");

0 commit comments

Comments
 (0)