[lworld] RFR: 8253113: [lworld] [lw3] C1 should avoid copying element of flattened arrays when reading a sub-element

[Tobias Hartmann]

On Mon, 14 Sep 2020 13:55:17 GMT, Frederic Parain <fparain at openjdk.org> wrote:

> Please review these changes optimizing access to flattened arrays in C1.
> 
> The optimization avoids unnecessary copies of intermediate values when the code accesses a sub-element of a flattened
> array.
> For instance, with the following code:
> 
> inline class Point { int x = 0, y = 0; }
> void foo() {
>     Point[] array = new Point[10];
>     int i = array[1].x;
> }
> 
> C1 used to create a new instance of Point, copy the content from the array, then reads the x field from this new
> instance. With the optimization, C1 directly accesses the x field from the flattened array, and makes no heap
> allocation.
> A simple benchmark on the "int i = array[1].x;" line gives these results:
> 
> baseline:
> Benchmark                          Mode  Samples  Score  Score error  Units
> o.s.MyBenchmark.testArrayReads     avgt      200  4.250        0.011  ns/op
> 
> optimized:
> Benchmark                          Mode  Samples  Score  Score error  Units
> o.s.MyBenchmark.testArrayReads     avgt      200  2.228        0.004  ns/op
> 
> Thank you,
> 
> Fred

Looks good to me, added some minor comments.

src/hotspot/share/c1/c1_LIRGenerator.cpp line 1605:

> 1603:   assert(flat_array_klass->is_loaded(), "must be");
> 1604:
> 1605:   ciInlineKlass* elem_klass = NULL;

Unused.

src/hotspot/share/c1/c1_LIRGenerator.cpp line 1611:

> 1609:   BasicType subelt_type = field->type()->basic_type();
> 1610:
> 1611:   #ifndef _LP64

Wrong indentation.

src/hotspot/share/c1/c1_LIRGenerator.cpp line 1598:

> 1596: };
> 1597:
> 1598: void LIRGenerator::access_sub_element(LIRItem& array, LIRItem& index, LIR_Opr& result, ciField* field, int
> sub_offset) {

There is quite some code duplication here with access_flattened_array, can we factor it out?

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

Marked as reviewed by thartmann (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/187