On Wed, 13 May 2026 13:20:08 GMT, Marc Chevalier <[email protected]> wrote:

>> As suggested by Tobias, it seems that the loop
>> https://github.com/openjdk/valhalla/blob/de6aff4ffe3344bfb886fa27385e432016bbbeef/src/hotspot/share/opto/graphKit.cpp#L2344-L2347
>> is wrong, even with the index (`[0]`) fixed.
>> 
>> Indeed, `final_state` is the `SafePointNode` that tracks the JVM state, that 
>> is the stack, slots... This kind of things. There is no good reason this 
>> should replace the output of the existing call. Instead, we should replace 
>> the projection of the old call with the output of the new call. The new call 
>> can be found as being the input of `result`. Of course, `result` can be 
>> something else than a projection from a call, typically a load, a 
>> constant... But then, I don't think we can be in the case of an unloaded 
>> return type. Testing seems to agree with that.
>> 
>> Another thing to take care of: `callprojs->resproj[i]` might be null, when 
>> the projection isn't used. Indeed, `store_inline_type_fields_to_buf` has a 
>> lot of `double` input, and so the `Half` part of these inputs are `top` (and 
>> thus, not wired from the old call). On the other hand, the projection of the 
>> new call must be there, but they should:
>> https://github.com/openjdk/valhalla/blob/de6aff4ffe3344bfb886fa27385e432016bbbeef/src/hotspot/share/opto/graphKit.cpp#L2167-L2181
>> 
>> Thanks,
>> Marc
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Marc Chevalier has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fewer flags

Thanks @TobiHartmann for review!

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

PR Comment: https://git.openjdk.org/valhalla/pull/2434#issuecomment-4441575044

Reply via email to