[lworld] RFR: 8374122: [lworld] compiler/c2/cmove/TestScalarConditionalMoveCmpObj.java fails with ir mismatches when preview enabled, post jdk-27+3
Marc Chevalier
mchevalier at openjdk.org
Tue Jan 20 11:12:58 UTC 2026
The test `TestScalarConditionalMoveCmpObj.java` tests whether code such as
r[i] = (a[i] != b[i]) ? cc : dd;
where `a` and `b` are `Object[]` gives a `CMoveNode`. Without Valhalla, we get a simple `If -> If(True|False) -> Region` diamond
<img width="2047" height="1382" alt="diamond" src="https://github.com/user-attachments/assets/811d4a51-6c0e-4789-bf37-3b1fa19ba0f5" />
that is simplified as a CMove.
<img width="2156" height="1061" alt="cmove" src="https://github.com/user-attachments/assets/f1c3277d-57e6-4af4-be3d-2ec52da7e5bc" />
Only this simple shape is simplified:
https://github.com/openjdk/valhalla/blob/a4fb7ebd5af316d3e99d10bffb44d5be3aab5548/src/hotspot/share/opto/loopopts.cpp#L723-L730
But with Valhalla, since operands could be value objects, the logic is a lot more complex: are pointer equal? If not, are one of both operands null? If both aren't null, are the operand the same class? If so... All being eventually concluded with a call to `isSubstitutable`. This can't be simplified as a `CMove` (or not easily).
Even warm up isn't enough: while `a[i]` and `b[i]` are guessed to be exactly `Object`, this still requires a more complex logic than a diamond. Schematically: are pointers equal? If not, is lhs null (that is enough to conclude they are different objects)? If so, trap; if not, is lhs of the speculated (non-value) type? If so, they are different; if not, trap. That is too complicated to make a `CMove`.
A solution is to make sure we can prove, rather than speculate, that operands are not value objects. Instead of making `a` and `b` arrays of `Object` that can be value types at runtime, we make them arrays of a non-abstract non-value type. This cannot be derived into a value class, so it's fine. And the test pass again!
I've also done a bit of side fixup in these tests: fixing package name and call `init` on the arrays used for the `int` flavor of the test (method `private static void TestScalarConditionalMoveCmpObj::init(int[] a)` was suspiciously unused).
Thanks,
Marc
-------------
Commit messages:
- Unproblem-list
- copyright
- Use another class so C2 knows it's not a value class
Changes: https://git.openjdk.org/valhalla/pull/1934/files
Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1934&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8374122
Stats: 29 lines in 4 files changed: 10 ins; 1 del; 18 mod
Patch: https://git.openjdk.org/valhalla/pull/1934.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1934/head:pull/1934
PR: https://git.openjdk.org/valhalla/pull/1934
More information about the valhalla-dev
mailing list