Skip to content

Commit a156447

Browse files
Roman MarchenkoPaul Hohensee
authored andcommitted
8358751: C2: Recursive inlining check for compiled lambda forms is broken
Backport-of: 9cca4f7c760bea9bf79f7c03f37a70449acad51e
1 parent cbc605a commit a156447

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; }
@@ -1313,7 +1330,7 @@ void CallLeafNode::dump_spec(outputStream *st) const {
13131330

13141331
//=============================================================================
13151332

1316-
void SafePointNode::set_local(JVMState* jvms, uint idx, Node *c) {
1333+
void SafePointNode::set_local(const JVMState* jvms, uint idx, Node *c) {
13171334
assert(verify_jvms(jvms), "jvms must match");
13181335
int loc = jvms->locoff() + idx;
13191336
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;
@@ -373,7 +377,7 @@ class SafePointNode : public MultiNode {
373377
}
374378

375379
private:
376-
void verify_input(JVMState* jvms, uint idx) const {
380+
void verify_input(const JVMState* jvms, uint idx) const {
377381
assert(verify_jvms(jvms), "jvms must match");
378382
Node* n = in(idx);
379383
assert((!n->bottom_type()->isa_long() && !n->bottom_type()->isa_double()) ||
@@ -382,34 +386,44 @@ class SafePointNode : public MultiNode {
382386

383387
public:
384388
// Functionality from old debug nodes which has changed
385-
Node *local(JVMState* jvms, uint idx) const {
386-
verify_input(jvms, jvms->locoff() + idx);
387-
return in(jvms->locoff() + idx);
389+
Node* local(const JVMState* jvms, uint idx) const {
390+
uint loc_idx = jvms->locoff() + idx;
391+
assert(jvms->is_loc(loc_idx), "not a local slot");
392+
verify_input(jvms, loc_idx);
393+
return in(loc_idx);
388394
}
389-
Node *stack(JVMState* jvms, uint idx) const {
390-
verify_input(jvms, jvms->stkoff() + idx);
391-
return in(jvms->stkoff() + idx);
395+
Node* stack(const JVMState* jvms, uint idx) const {
396+
uint stk_idx = jvms->stkoff() + idx;
397+
assert(jvms->is_stk(stk_idx), "not a stack slot");
398+
verify_input(jvms, stk_idx);
399+
return in(stk_idx);
392400
}
393-
Node *argument(JVMState* jvms, uint idx) const {
394-
verify_input(jvms, jvms->argoff() + idx);
401+
Node* argument(const JVMState* jvms, uint idx) const {
402+
uint arg_idx = jvms->argoff() + idx;
403+
assert(jvms->is_stk(arg_idx), "not an argument slot");
404+
verify_input(jvms, arg_idx);
395405
return in(jvms->argoff() + idx);
396406
}
397-
Node *monitor_box(JVMState* jvms, uint idx) const {
407+
Node* monitor_box(const JVMState* jvms, uint idx) const {
398408
assert(verify_jvms(jvms), "jvms must match");
399-
return in(jvms->monitor_box_offset(idx));
409+
uint mon_box_idx = jvms->monitor_box_offset(idx);
410+
assert(jvms->is_monitor_box(mon_box_idx), "not a monitor box offset");
411+
return in(mon_box_idx);
400412
}
401-
Node *monitor_obj(JVMState* jvms, uint idx) const {
413+
Node* monitor_obj(const JVMState* jvms, uint idx) const {
402414
assert(verify_jvms(jvms), "jvms must match");
403-
return in(jvms->monitor_obj_offset(idx));
415+
uint mon_obj_idx = jvms->monitor_obj_offset(idx);
416+
assert(jvms->is_mon(mon_obj_idx) && !jvms->is_monitor_box(mon_obj_idx), "not a monitor obj offset");
417+
return in(mon_obj_idx);
404418
}
405419

406-
void set_local(JVMState* jvms, uint idx, Node *c);
420+
void set_local(const JVMState* jvms, uint idx, Node *c);
407421

408-
void set_stack(JVMState* jvms, uint idx, Node *c) {
422+
void set_stack(const JVMState* jvms, uint idx, Node *c) {
409423
assert(verify_jvms(jvms), "jvms must match");
410424
set_req(jvms->stkoff() + idx, c);
411425
}
412-
void set_argument(JVMState* jvms, uint idx, Node *c) {
426+
void set_argument(const JVMState* jvms, uint idx, Node *c) {
413427
assert(verify_jvms(jvms), "jvms must match");
414428
set_req(jvms->argoff() + idx, c);
415429
}

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)