RFR: 8375653: C2: CmpUNode::sub is not monotonic [v4]
Emanuel Peter
epeter at openjdk.org
Thu Jan 22 14:32:19 UTC 2026
On Thu, 22 Jan 2026 13:46:51 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Hi,
>>
>> This PR fixes the issue that `CmpUNode::sub` is not monotonic. The root cause is that it returns different values for several cases, but the cases are not mutually exclusive and the return values are not a subset of each other. This leads to the possibilities that a node satisfying both cases will return the first value, but if upon being widen it ceases to satisfy the first case but still satisfies the second case, the method will return the second value, which is not a superset of the previous result.
>>
>> For example, given `r = CmpU(x, y)`.
>>
>> At the first iteration, `type(x) = {0}` and `type(y) = {1, -1}`, then `CmpUNode::sub` returns `TypeInt::CC_LE` since it sees that `x` is the constant `0`.
>>
>> At the second iteration, `type(x) = {0, 2}` and `type(y) = {-1, 1}`, then `CmpUNode::sub` returns `TypeInt::CC_NE` since it sees that `x` and `y` do not overlap. This is not a superset of `TypeInt::CC_LE`, which leads to an assertion.
>>
>> Please take a look and leave your reviews, thanks a lot.
>
> Quan Anh Mai has updated the pull request incrementally with two additional commits since the last revision:
>
> - Missing patterns
> - Add IR tests, fix correctness tests
Nice, thanks for adding some IR tests!
test/hotspot/jtreg/compiler/c2/gvn/CmpUNodeValueTests.java line 59:
> 57: Random r = Utils.getRandomInstance();
> 58: long x = r.nextLong();
> 59: long y = r.nextLong();
If you use `Generators.java` you may be more likely to hit interesting edge cases (zero, max_uint, etc).
test/hotspot/jtreg/compiler/c2/gvn/CmpUNodeValueTests.java line 86:
> 84:
> 85: @Test
> 86: @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
Just as a "control test", can you add a comparison that does not fold away, and so we should find the nodes?
Otherwise, the risk is that we do `failOn` for the wrong nodes and don't notice.
-------------
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/29308#pullrequestreview-3692707141
PR Review Comment: https://git.openjdk.org/jdk/pull/29308#discussion_r2717124928
PR Review Comment: https://git.openjdk.org/jdk/pull/29308#discussion_r2717134137
More information about the hotspot-compiler-dev
mailing list