[master] RFR: Relax array elements alignment [v5]

[Thomas Stuefe]

On Wed, 20 Apr 2022 14:15:56 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> PR #40 eliminated the Klass* word. A whole class of objects would not benefit from it, though: array elements are aligned at 8-byte-boundaries, even for element types where this is not necessary (byte, bool, char, short, int, float, compressed-objs). This means that array elements would still start at byte#16, after 8 bytes header and 4 bytes length and another 4 bytes unused gap.
>> 
>> This change improves the array element alignment and relaxes it such that elements can align at 4-byte boundaries.
>> 
>> Note about C2 changes: the code that extracts the state field (either byte, int or long array) assumed that all arrays would be equal, and asks for header size of T_INT. This would give wrong offset for T_LONG arrays.
>> 
>> I'm adding a test that checks correct array elements base offsets.
>> 
>> GHA seems to see 3 new test failures (timeouts), but only on Windows(x64). I ran the same test on a different Windows machine (all of compiler/c2/irTests) and could not reproduce the failures. They do look like timeouts on GHA, maybe it's just too long-running tests?
>> 
>> Those are the GHA/Windows failing tests:
>> compiler/c2/irTests/TestAutoVectorization2DArray.java 
>> compiler/c2/irTests/TestIRAbs.java 
>> compiler/c2/irTests/TestStripMiningDropsSafepoint.java 
>> 
>> Testing:
>>  - [x] tier1 (x86_64)
>>  - [x] tier1 (x86_32)
>>  - [x] tier1 (aarch64)
>>  - [x] tier2 (x86_64)
>>  - [x] tier2 (x86_32)
>>  - [x] tier2 (aarch64)
>>  - [x] tier3 (x86_64)
>>  - [x] tier3 (x86_32)
>>  - [x] tier3 (aarch64)
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use correct array element type to determine header size in TypeAryPtr::dump2()

Hi Roman,

looks okay overall, small remarks inline. 

Cheers, Thomas

src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp line 187:

> 185: 
> 186:   // Zero first 4 bytes, if start offset is not 64bit aligned.
> 187:   if ((hdr_size_in_bytes & (BytesPerWord - 1)) != 0) {

Does not do what the comment says, since BytesPerWord=4 on 32-bit.

On 32-bit I believe this condition is always false.

Maybe use BytesPerLong instead? Also, maybe use is_aligned()?


!is_aligned(hdr_size_in_bytes, BytesPerLong)

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 3791:

> 3789:   xorptr(temp, temp);    // use _zero reg to clear memory (shorter code)
> 3790: #ifdef _LP64
> 3791:   if ((offset_in_bytes & (BytesPerWord - 1)) != 0) {

Did the old code ever bother to zero out the gap between array size and first element address in compressed class pointers off mode?

I'd prefer BytesPerLong here too.

src/hotspot/share/oops/arrayOop.hpp line 132:

> 130:   // Returns the header size in bytes aligned to the requirements of the
> 131:   // array object type.
> 132:   static int header_size_in_bytes(BasicType type) {

Side remark, I wonder whether "first_element_offset" or something similar would not be more descriptive.

src/hotspot/share/oops/objArrayOop.hpp line 51:

> 49: 
> 50: private:
> 51:   // Give size of objArrayOop in HeapWords minus the header

Comment outdated

test/hotspot/jtreg/gc/g1/plab/TestPLABPromotion.java line 76:

> 74:     private static final int OBJECT_SIZE_SMALL = 10;
> 75:     private static final int OBJECT_SIZE_MEDIUM = 100;
> 76:     private static final int OBJECT_SIZE_HIGH = Platform.is64bit() ? 3266 : 3258;

why this difference?

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

PR: https://git.openjdk.java.net/lilliput/pull/41