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

[Vladimir Ivanov]

On Fri, 19 Mar 2021 02:15:03 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.
>> 
>> I adjust my codes and add a new benchmark
>> 
>> public class MyBenchmark {
>> 
>>     static int[] data = new int[10000];
>> 
>>     static {
>>         for(int i = 0; i < data.length; ++i) {
>>             data[i] = i * 1337 % 7331;
>>         }
>>     }
>> 
>>     @Benchmark
>>     public void testMethod(Blackhole bh) {
>>       int sum = 0;
>>       for (int i = 0; i < data.length; i++) {
>>           Integer ii = Integer.valueOf(data[i]);
>>           black();
>>           if (i < 100000) {
>>               sum += ii.intValue();
>>           }
>>       }
>>       bh.consume(sum);
>>     }
>> 
>>     public void black(){}
>> }
>> 
>> 
>> aarch64:  
>> base line：
>> Benchmark                     Mode  Samples   Score  Score error  Units
>> o.s.MyBenchmark.testMethod    avgt       30  88.513        1.111  us/op
>> 
>> opt：
>> Benchmark                     Mode  Samples   Score  Score error  Units
>> o.s.MyBenchmark.testMethod    avgt       30  52.776        0.096  us/op
>> 
>> x86:  
>> base line：
>> Benchmark                     Mode  Samples   Score  Score error  Units
>> o.s.MyBenchmark.testMethod    avgt       30  81.066        3.156  us/op
>> 
>> opt:
>> Benchmark                     Mode  Samples   Score  Score error  Units
>> o.s.MyBenchmark.testMethod    avgt       30  55.596        0.775  us/op
>
> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   refactor

Looks good.

Some more minor cleanup suggestions follow.

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

> 568: static void scalarize_debug_usages(CallNode* call, Node* resproj) {
> 569:   Unique_Node_List safepoints;
> 570:   for (DUIterator_Fast imax, i = resproj->fast_outs(imax); i < imax; i++) {

No need to collect safepoints anymore. You can process users while iterating over them.
for (DUIterator_Last imin, i = res->last_outs(imin); i >= imin;) {
  SafePointNode* sfpt = res->last_out(i)->as_SafePoint();
  ...
  int num_edges = sfpt->replace_edges_in_range(res, sobj, start, end);  
  i -= num_edges;
}

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

> 574: 
> 575: #ifndef PRODUCT
> 576:   if (PrintEliminateAllocations && safepoints.size() > 0) {

`safepoints.size() > 0` is redundand.

Also, I'd move the diagnostic logic to the end of the method after all the users are processed.

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

> 588:     Node* sfpt = safepoints.pop();
> 589: 
> 590:     ciInstanceKlass* klass = call->as_CallStaticJava()->method()->holder();

`klass` is loop invariant and can be moved out of the loop (along with `n_fields`).

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

> 595:     Node* sobj = new SafePointScalarObjectNode(gvn.type(res)->isa_oopptr(),
> 596: #ifdef ASSERT
> 597:                                                   call->isa_Allocate(),

It is always `NULL` since `call` can't be an `Allocate`.

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

Marked as reviewed by vlivanov (Reviewer).

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