On Wed, 11 Sep 2024 13:34:28 GMT, Roman Kennke <[email protected]> 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.
>
> I will re-evaluate those tests, and add comments or remove the restrictions.
If it has indeed been a while, then it might well be that some of them work
now, since I did make some improvements to auto-vectorization ;)
My suggestion is this: go over the examples, check which ones are now ok. Those
that are not ok, add a comment, and file a bug: I can then analyze those cases
later, and see how to write other tests or improve auto-vectorization.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20677#issuecomment-2343797957
