On Wed, 22 Apr 2026 12:26:00 GMT, Benoît Maillard <[email protected]> wrote:
>> This PR prevents a missed optimization caused by a breach of idempotence in
>> `InlineTypeNode::Value` when handling contradicting method return profiles.
>>
>> ## Conflicting return profiles
>>
>> Suppose we have the following java code, and that we have record profile
>> data for both callsites (`call(...)` and `callWrapper(...)`):
>>
>>
>> static MyValue callWrapper(...) {
>> return call(...); // callsite #1
>> }
>>
>> static MyValue test() {
>> return callWrapper(...); // callsite #2
>> }
>>
>>
>> It could be that `test` is not the only place where we call
>> `callWrapper(...)`, and that the other call sites result in wildy different
>> executions of callsite `#1` with wildly different return values at that
>> callsite.
>>
>> Suppose that callsite `#1` always returns `null`, except when `callWrapper`
>> is called from callsite `#2` where it throws an exception. In
>> `ciMethod::return_profiled_type`, we end up with the following
>> `ProfilePtrKind` when compiling `test`:
>> - For callsite `#1`: `ProfileAlwaysNull`, because we have only observed
>> `null` (the rest of the time an exception was thrown, so we didn't see
>> anything)
>> - For callsite `#2`: `ProfileNeverNull`, because we have never observed a
>> return value and this is basically the default
>>
>> Please look at the test from this PR for a more complete example.
>>
>> These two profiles contradict each other. If we do incremental inlining
>> (probably not limited to it, but easier to reproduce), we can easily end up
>> in a situation where a speculative type derived from the return of the
>> callsite is `NotNull` before inlining the outer callsite, and then moves to
>> `ptr:null` after inlining. This can confuse `Value` implementations that
>> rely on `filter_speculative`. This is handled gracefully for
>> `ConstraintCastNode`, but not for `InlineTypeNode`.
>>
>> ## Missed optimization
>>
>> In `InlineTypeNode::Value`, we have:
>> https://github.com/openjdk/valhalla/blob/e391f76e7db834f50c1a0af466daf95a65a79f23/src/hotspot/share/opto/inlinetypenode.cpp#L1619
>>
>> Suppose we have the following types (only the speculative part is shown):
>>
>> oop:
>> instptr:compiler/valhalla/inlinetypes/TestPollutedCallsiteProfileInlineType$MyValue:NotNull:exact
>> *,iid=bot (flat in array)
>> _type:
>> instptr:compiler/valhalla/inlinetypes/TestPollutedCallsiteProfileInlineType$MyValue:NotNull:exact
>> *,iid=bot (flat in array)
>>
>>
>> After this step we have `t = oop = _type`.
>>
>> The oop input suddenly changes types because of incremental inlining, as
>> explained in the previ...
>
> Benoît Maillard has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Reduce test iterations
Thanks, the fix makes sense to me.
-------------
Marked as reviewed by qamai (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/2351#pullrequestreview-4155576135
