Skip to content
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

8261137: Optimization of Box nodes in uncommon_trap #2401

Closed
wants to merge 13 commits into from
@@ -554,6 +554,53 @@ void LateInlineVirtualCallGenerator::do_late_inline() {
CallGenerator::do_late_inline_helper();
}

static void delay_box_in_uncommon_trap(CallNode* call, Node* resproj) {
if (resproj != NULL && call->is_CallStaticJava() &&
This conversation was marked as resolved by Wanghuang-Huawei

This comment has been minimized.

@navyxliu

navyxliu Feb 9, 2021
Contributor

IMHO, we should use nullptr here because hotspot now is using c++14.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Feb 9, 2021
Author Contributor

Thank you for your review. I will change that.

call->as_CallStaticJava()->is_boxing_method()) {
GraphKit kit(call->jvms());
This conversation was marked as resolved by Wanghuang-Huawei

This comment has been minimized.

@navyxliu

navyxliu Feb 9, 2021
Contributor

you can postpone to construct this object in if(no_use).

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Feb 9, 2021
Author Contributor

Sure. I will change that.

PhaseGVN& gvn = kit.gvn();

Node_List delay_boxes;
bool no_use = true;
for (DUIterator_Fast imax, i = resproj->fast_outs(imax); i < imax; i++) {
Node* m = resproj->fast_out(i);
if (m->is_CallStaticJava() &&
m->as_CallStaticJava()->uncommon_trap_request() != 0) {
delay_boxes.push(m);
} else {
no_use = false;
break;
}
}

if (no_use) {
// delay box node in uncommon_trap runtime, treat box as a scalarized object
while (delay_boxes.size() > 0) {
ProjNode* res = resproj->as_Proj();
Node* uncommon_trap_node = delay_boxes.pop();
int in_edge = uncommon_trap_node->find_edge(res);
assert(in_edge > 0, "sanity");
This conversation was marked as resolved by Wanghuang-Huawei

This comment has been minimized.

@vnkozlov

vnkozlov Feb 8, 2021
Contributor

If there are several references you need to replace all of them.

This comment has been minimized.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Feb 9, 2021
Author Contributor

Thank you for your review. It's my fault. I will revise this in next commit.


ciInstanceKlass* klass = call->as_CallStaticJava()->method()->holder();
int n_fields = klass->nof_nonstatic_fields();
assert(n_fields == 1, "sanity");
This conversation was marked as resolved by Wanghuang-Huawei

This comment has been minimized.

@navyxliu

navyxliu Feb 9, 2021
Contributor

I think you also need to check the only non-static field of klass must be a scalar.
"sanity" is too concise. I think we should leave a message to say it's an auto-boxing class.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Feb 27, 2021
Author Contributor

Yes. I will revise that. Thank you.


uint first_ind = (uncommon_trap_node->req() - uncommon_trap_node->jvms()->scloff());
Node* sobj = new SafePointScalarObjectNode(gvn.type(res)->isa_oopptr(),
#ifdef ASSERT
NULL,
This conversation was marked as resolved by Wanghuang-Huawei

This comment has been minimized.

@vnkozlov

vnkozlov Feb 8, 2021
Contributor

I would suggest to record call node here treating it as allocation.

This comment has been minimized.

@Wanghuang-Huawei

Wanghuang-Huawei Feb 27, 2021
Author Contributor

The prototype of the constuctor is SafePointScalarObjectNode(const TypeOopPtr* , AllocateNode* , uint, uint). However , call here is a CallStaticJavaNode * instead of AllocateNode *. Do you mean that we just pass the call as a record here?

#endif // ASSERT
first_ind, n_fields);
sobj->init_req(0, kit.root());
uncommon_trap_node->add_req(call->in(res->_con));
sobj = gvn.transform(sobj);
uncommon_trap_node->jvms()->set_endoff(uncommon_trap_node->req());
uncommon_trap_node->set_req(in_edge, sobj);
}
}
}
}

void CallGenerator::do_late_inline_helper() {
assert(is_late_inline(), "only late inline allowed");

@@ -603,50 +650,7 @@ void CallGenerator::do_late_inline_helper() {
C->remove_macro_node(call);
}

if (callprojs.resproj != NULL && call->is_CallStaticJava() &&
call->as_CallStaticJava()->is_boxing_method()) {
GraphKit kit(call->jvms());
PhaseGVN& gvn = kit.gvn();

Node_List delay_boxes;
bool no_use = true;
for (DUIterator_Fast imax, i = callprojs.resproj->fast_outs(imax); i < imax; i++) {
Node* m = callprojs.resproj->fast_out(i);
if (m->is_CallStaticJava() &&
m->as_CallStaticJava()->uncommon_trap_request() != 0) {
delay_boxes.push(m);
} else {
no_use = false;
break;
}
}

if (no_use) {
// delay box node in uncommon_trap runtime, treat box as a scalarized object
while (delay_boxes.size() > 0) {
ProjNode* res = callprojs.resproj->as_Proj();
Node* uncommon_trap_node = delay_boxes.pop();
int in_edge = uncommon_trap_node->find_edge(res);
assert(in_edge > 0, "sanity");

ciInstanceKlass* klass = call->as_CallStaticJava()->method()->holder();
int n_fields = klass->nof_nonstatic_fields();
assert(n_fields == 1, "sanity");

uint first_ind = (uncommon_trap_node->req() - uncommon_trap_node->jvms()->scloff());
Node* sobj = new SafePointScalarObjectNode(gvn.type(res)->isa_oopptr(),
#ifdef ASSERT
NULL,
#endif // ASSERT
first_ind, n_fields);
sobj->init_req(0, C->root());
uncommon_trap_node->add_req(call->in(res->_con));
sobj = gvn.transform(sobj);
uncommon_trap_node->jvms()->set_endoff(uncommon_trap_node->req());
uncommon_trap_node->set_req(in_edge, sobj);
}
}
}
delay_box_in_uncommon_trap(call, callprojs.resproj);

bool result_not_used = (callprojs.resproj == NULL || callprojs.resproj->outcnt() == 0);
if (is_pure_call() && result_not_used) {
ProTip! Use n and p to navigate between commits in a pull request.