RFR: 8375653: C2: CmpUNode::sub is not monotonic
Dean Long
dlong at openjdk.org
Thu Jan 22 04:42:47 UTC 2026
On Thu, 22 Jan 2026 00:16:43 GMT, Dean Long <dlong 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.
>
> Would it make sense to have stand-alone C++ tests for these and maybe other interesting cases? Maybe using the gtest framework?
> @dean-long I guess it is possible, but the implementation is simple enough that the test will just be a repetition of the implementation. Furthermore, doing so also requires some non-trivial refactoring since `CmpUNode::sub` inspects the structure around the node, not just the `Type` of its inputs. What do you think?
Good point. I'm hoping we can get rid of node structure inspection, then we could make these static functions that only deal with the type. Even if it turns out that the is_index_range_check() code is still needed, we could make CmpULNode::sub static and refactor CmpUNode::sub to have a static helper that can be tested stand-alone.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/29308#issuecomment-3782469169
More information about the hotspot-compiler-dev
mailing list