RFR: 8349479: C2: when a Type node becomes dead, make CFG path that uses it unreachable [v3]
Christian Hagedorn
chagedorn at openjdk.org
Mon Mar 31 12:03:37 UTC 2025
On Mon, 31 Mar 2025 10:17:51 GMT, Roland Westrelin <roland at openjdk.org> wrote:
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23468#discussion_r2020909000
More information about the hotspot-compiler-dev
mailing list