RFR: 8359602: Ideal optimizations depending on input type are missed because of missing notification mechanism from CCP [v2]

[Emanuel Peter]

On Tue, 1 Jul 2025 07:07:40 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

> > @benoitmaillard One more open question for me: `raise_bottom_type` only sets the node internal `_type`. But in IGVN, we do not read from `_type` but `phase->type(in(2))`. Do you know when the `phase->type(in(2))` value changes? Is that also during CCP? Before or after the `_type` is modified?
> 
> Yes, good point, I should I have mentioned this somewhere. The `phase->type(in(2))` call uses the type array from `PhaseValues`. The type array entry is actually modified earlier, in `PhaseCCP::analyze`, right after the `Value` call. You can see the `set_type` call [here](https://github.com/benoitmaillard/jdk/blob/75de51dff6d9cc3e9764737b29b9358992b488b7/src/hotspot/share/opto/phaseX.cpp#L2765). When this happens, users are added to the (local) worklist but again it does not change our issue as only value optimizations occur in that context.

Thanks for the explanation! So it seems that `CCP` and `IGVN` share the type array, right? Ah yes, it is the `Compile::_types`:

   461   // Shared type array for GVN, IGVN and CCP. It maps node idx -> Type*. 
   462   Type_Array*           _types;


If the value behind `phase->type(in(2))` (the type array entry) is modified in `PhaseCCP::analyze`, right after the `Value` call, then why not do the notification there? If we did that, we would do more notification than what you now proposed (to do the notification in `PhaseCCP::transform_once` on the nodes that have a type that is different than the `bottom_type`). Are we possibly missing any important case with your approach now? Probably not, I would argue: with your approach we still notify for all live nodes that have a modified type, or are replaced with a constant. If we notified after every type update in `PhaseCCP::analyze`, we might notify for nodes multiple times, and we would also notify for nodes that are dead after CCP - both are unnecessary overheads. Alright, I just wanted to think this through - but it seems your approach is good :)

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

PR Comment: https://git.openjdk.org/jdk/pull/26017#issuecomment-3023637471