RFR: 8261137: Optimization of Box nodes in uncommon_trap [v5]

[Wang Huang]

On Wed, 24 Feb 2021 07:06:34 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add debuginfo optimization
>
> src/hotspot/share/opto/callGenerator.cpp line 561:
> 
>> 559:   if (resproj != nullptr && call->is_CallStaticJava() &&
>> 560:       call->as_CallStaticJava()->is_boxing_method()) {
>> 561:     Unique_Node_List debuginfo_node_list;
> 
> Maybe rename this to `safepoints`.

Sure.

> src/hotspot/share/opto/callGenerator.cpp line 569:
> 
>> 567:         for (uint i = 0; i < dbg_start; i++) {
>> 568:           if (sfpt->in(i) == resproj) {
>> 569:             return;
> 
> I think this code can be replaced by:
>   if (!sfpt->is_Call() || !sfpt->as_Call()->has_non_debug_use(n)) {
>     safepoints.push(sfpt);
>   } else {
>     ...
>   }

Yes, I will do that.

> src/hotspot/share/opto/callGenerator.cpp line 656:
> 
>> 654:   }
>> 655: 
>> 656:   replace_box_to_scalar(call, callprojs.resproj);
> 
> Should this be guarded by `C->eliminate_boxing()`?

Exactly. I will change that.

> src/hotspot/share/opto/callnode.hpp line 503:
> 
>> 501:                      // It is relative to the last (youngest) jvms->_scloff.
>> 502:   uint _n_fields;    // Number of non-static fields of the scalarized object.
>> 503:   bool _is_auto_box; // is the scalarized object is auto box.
> 
> Typo in comment. Should be something like `// True if the scalarized object is an auto box`

Thank you for your review. I'll add this in my next patch.

> src/hotspot/share/opto/callGenerator.cpp line 583:
> 
>> 581:     while (debuginfo_node_list.size() > 0) {
>> 582:       ProjNode* res = resproj->as_Proj();
>> 583:       Node* debuginfo_node = debuginfo_node_list.pop();
> 
> `debuginfo_node` -> `safepoint`

OK.

> src/hotspot/share/opto/callGenerator.cpp line 596:
> 
>> 594:                                                   first_ind, n_fields, true);
>> 595:       sobj->init_req(0, kit.root());
>> 596:       debuginfo_node->add_req(call->in(res->_con));
> 
> I don't understand why you are selecting the input based on the result projection field `res->_con`?

Thank you for your review. I use `sfpt->add_req(call->in(TypeFunc::Parms));` instead of this.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2401