RFR: 8340010: Fix vectorization tests with compact headers

[Hamlin Li]

On Mon, 18 Nov 2024 10:32:44 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> This is a followup to:
> https://github.com/openjdk/jdk/pull/20677 / [JDK-8305895](https://bugs.openjdk.org/browse/JDK-8305895) Implement JEP 450: Compact Object Headers (Experimental)
> 
> @rkennke fixed the vectorization tests in a very rudamentary way. I now took some time to see what exactly is affected. At the time I reviewed the JEP, I thought only very minor cases were affected, like hand-unrolling etc. But it turns out that there are some more important cases that are affected, like **mixed-type loops**, such as **conversion** between types. Another class of affected loops is **hand-unrolled loops**.
> 
> **The problem is this:**
> Since the offset from array-base to payload has changed (16 -> 12), some vector loads/stores can now no longer be aligned. This means vectorization under `+AlignVector` is not possible.
> 
> --------------------------------
> 
> First: only platforms that require strict-alignment are affected (i.e. `+AlignVector` or `Matcher::misaligned_vectors_ok=false`). I filed [JDK-8344424](https://bugs.openjdk.org/browse/JDK-8344424) for this, so the impact can be discussed. **The affected platforms seem to be exceptions**.
> 
> -------------------
> 
> Now on to fixing the tests, which we need to do now anyway. Some actually were currently failing.
> 
> I once ran over `tier1,tier2,tier3,tier4` plus our internal stress testing with `+-AlignVector` and `+-UseCompactObjectHeaders`, collected all failing tests. I also looked at all tests that were already guarding IR rules on `UseCompactObjectHeaders`.
> 
> To almost all tests I added runs with `+-AlignVector` and `+-UseCompactObjectHeaders`. We could leave this also to global runs with these flag combinations. But it is rare that we ever run this, so I thought I want to directly run the "interesting" tests with all combinations. This requires extra test runtime, but I think it is warranted.
> 
> In a few cases I also added stronger IR rules, in tests that were already affected - just to make sure we have the behavior we want. Some cases would not vectorize for other cases, and I put comments there for future reference.

Thanks for the patch and comments in the code, it's much helpful.
Looks good, just some minor comments.

test/hotspot/jtreg/compiler/c2/irTests/TestVectorizationMismatchedAccess.java line 184:

> 182:     @IR(counts = { IRNode.LOAD_VECTOR_L, ">=1", IRNode.STORE_VECTOR, ">=1" },
> 183:         // This test fails with compact headers, but only with UseSSE<=3.
> 184:         applyIf = { "UseCompactObjectHeaders", "false" },

Is this removed intentiionally? or it should be `applyIfOr = {"UseCompactObjectHeaders", "false", "AlignVector", "false"},`?

test/hotspot/jtreg/compiler/c2/irTests/TestVectorizationMismatchedAccess.java line 196:

> 194:     @IR(counts = { IRNode.LOAD_VECTOR_L, ">=1", IRNode.STORE_VECTOR, ">=1" },
> 195:         // This test fails with compact headers, but only with UseSSE<=3.
> 196:         applyIf = { "UseCompactObjectHeaders", "false" },

same as above comment.

test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java line 111:

> 109:     // ---------------- Integer Extension ----------------
> 110:     @Test
> 111:     @IR(failOn = {IRNode.STORE_VECTOR})

Not sure if it's necessary to fail on the Node, I'm not an expert, so just a question, not a review comment. Similar question for the below  `failOn`s

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

PR Review: https://git.openjdk.org/jdk/pull/22199#pullrequestreview-2450860349
PR Review Comment: https://git.openjdk.org/jdk/pull/22199#discussion_r1851736762
PR Review Comment: https://git.openjdk.org/jdk/pull/22199#discussion_r1851737491
PR Review Comment: https://git.openjdk.org/jdk/pull/22199#discussion_r1851755129