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

[Xin Liu]

On Fri, 5 Feb 2021 07:29:00 GMT, Wang Huang <whuang at openjdk.org> wrote:

>> JDK-8075052 has  removed useless autobox. However, in some cases, the box is still saved. For instance:
>> @Benchmark
>> public void testMethod(Blackhole bh) {
>>   int sum = 0;
>>   for (int i = 0; i < data.length; i++) {
>>       Integer ii = Integer.valueOf(data[i]);
>>       if (i < data.length) {
>>           sum += ii.intValue();
>>       }
>>   }
>>   bh.consume(sum);
>> }
>> Although the variable ii is only used at ii.intValue(), it cannot be eliminated as a result of being used by a hidden uncommon_trap.
>> The uncommon_trap is generated by the optimized "if", because its condition is always true.
>> 
>> We can postpone box in uncommon_trap in this situation. We treat box as a scalarized object by adding a SafePointScalarObjectNode in the uncommon_trap node,
>> and deleting the use of box:
>> 
>> There is no additional fail/error(s) of jtreg after this patch.
>
> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   refactor codes

src/hotspot/share/opto/callGenerator.cpp line 558:

> 556: 
> 557: static void delay_box_in_uncommon_trap(CallNode* call, Node* resproj) {
> 558:   if (resproj != NULL && call->is_CallStaticJava() &&

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

src/hotspot/share/opto/callGenerator.cpp line 560:

> 558:   if (resproj != NULL && call->is_CallStaticJava() &&
> 559:       call->as_CallStaticJava()->is_boxing_method()) {
> 560:     GraphKit kit(call->jvms());

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

src/hotspot/share/opto/callGenerator.cpp line 586:

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

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.

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

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