RFR: 8349479: C2: when a Type node becomes dead, make CFG path that uses it unreachable [v3]

[Roland Westrelin]

On Mon, 31 Mar 2025 12:00:02 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> There's a good chance that it can never be null. I think it's been considered good practice over the year to be particularly defensive about this (there must be other Ideal transformations where inputs can be cleared as the graph is transformed) and I tend to add checks for null inputs systematically.
>
>> I think it's been considered good practice over the year to be particularly defensive about this
> 
> Makes sense from a stability point of view. I'm wondering though if it's not a bug when the cast input is null at this point. Aren't there only few CFG nodes, like regions, where we set some inputs to null already? There is other code, for example in `ConvI2L::Ideal()`, that later accesses `in(1)` without null check:
> 
> https://github.com/openjdk/jdk/blob/1ec2177a6b25573732b902f76bb81dd1cdaf7edf/src/hotspot/share/opto/convertnode.cpp#L728
> 
> To be consistent, we would also need to add a check for the other accesses in the method or turn the null check into a bailout for the entire `Ideal()` method. If we agree that null is unexpected (or assume it should be), we might also want to add asserts accordingly. 
> 
> My concern is that most IGVN methods assume non-control inputs cannot be null where we normally expect a sane input. This is probably true but hard to prove. To be overally consistent, we should also consider adding bailout and assertion code there. While it's the safest solution, this could introduce a lot of new code, especially for multi input nodes, which also makes it harder to read. What are your thought about that?
> 
> Anyway, we don't need to make a decision as part of this PR on how we should generally handle inputs in IGVN method. It's fine if we only concentrate on the touched/new code here.

I agree that we would need to be consistent and that it makes little sense to add null checks in code that have been around forever and has never caused issues. Maybe we can somehow have igvn itself assert that every node it processes has a set of expected inputs non null? I suppose, every node type would need to define which of its inputs can be null, then.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23468#discussion_r2028262990