On Wed, 11 Sep 2024 08:24:16 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
> @rkennke Can you please explain the changes in these tests:
>
> ```
> test/hotspot/jtreg/compiler/c2/irTests/TestVectorizationMismatchedAccess.java
> test/hotspot/jtreg/compiler/c2/irTests/TestVectorizationNotRun.java
> test/hotspot/jtreg/compiler/loopopts/superword/TestAlignVector.java
> test/hotspot/jtreg/compiler/loopopts/superword/TestIndependentPacksWithCyclicDependency.java
> test/hotspot/jtreg/compiler/loopopts/superword/TestMulAddS2I.java
> ```
>
> You added these IR rule restriction: `@IR(applyIf =
> {"UseCompactObjectHeaders", "false"},`
>
> This means that if `UseCompactObjectHeaders` is enabled, vectorization seems
> to be impacted - that could be concerning because it has a performance impact.
>
> I have recently changed a few things in SuperWord, so maybe some of them can
> be removed, because they now vectorize anyway?
>
> Of course some special tests may just rely on `UseCompactObjectHeaders ==
> false` - but I would need some comments in the tests where you added it to
> justify why we add the restriction.
>
> Please also test this patch with the cross combinations of
> `UseCompactObjectHeaders` and `AlignVector` enabled and disabled (and add
> `VerifyAlignVector` as well).
IIRC (it has been a while), the problem is that with Lilliput (and also without
compact headers, but disabling compressed class-pointers
-UseCompressedClassPointers, but nobody ever does that), byte[] and long[]
start at different offsets (12 and 16, respectively). That is because with
compact headers, we are using the 4 bytes after the arraylength, but
long-arrays cannot do that because of alignment constraints. The impact is that
these tests don't work as expected, because vectorization triggers differently.
I don't remember the details, TBH, but I believe they would now generate
pre-loops, or some might even not vectorize at all. Those seemed to be
use-cases that did not look very important, but I may be wrong. It would be
nice to properly fix those tests, or make corresponding tests for compact
headers, instead, or improve vectorization to better deal with the offset
mismatch, if necessary/possible.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20677#issuecomment-2343693629
