RFR: 8347365: C2: Fix the handling of depends_only_on_test [v2]

[Quan Anh Mai]

On Fri, 16 Jan 2026 13:22:58 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - more clarification
>>  - Refine comments
>
> src/hotspot/share/opto/callnode.hpp line 123:
> 
>> 121:   virtual bool  is_CFG() const { return true; }
>> 122:   virtual uint hash() const { return NO_HASH; }  // CFG nodes do not hash
>> 123:   virtual bool depends_only_on_test() const { return false; }
> 
> Why are you not replacing this with an implementation for depends_only_on_test_impl()? Same question for `RethrowNode`, `GotoNode`, and `RegionNode`.

Because they are CFG nodes, `depends_only_on_test` returns `false` without the need to invoke `depends_only_on_test_impl`.

> src/hotspot/share/opto/loopnode.hpp line 1684:
> 
>> 1682:   // Mark an IfNode as being dominated by a prior test,
>> 1683:   // without actually altering the CFG (and hence IDOM info).
>> 1684:   void dominated_by(IfProjNode* prevdom, IfNode* iff, bool flip = false, bool pin_array_access_nodes = false);
> 
> Please also rename this `pin_array_access_nodes`, since it is just wired through to `rewire_safe_outputs_to_dominator()`

Good catch, I have renamed the parameter.

> src/hotspot/share/opto/memnode.hpp line 316:
> 
>> 314: 
>> 315: private:
>> 316:   // depends_only_on_test is almost always true, and needs to be almost always
> 
> Suggestion:
> 
>   // depends_only_on_test_impl is almost always true, and needs to be almost always

While the method being overriden is `depends_only_on_test_impl`, the method that is called is `depends_only_on_test`, so it makes more sense to reference the public API here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29158#discussion_r2699079336
PR Review Comment: https://git.openjdk.org/jdk/pull/29158#discussion_r2699086414
PR Review Comment: https://git.openjdk.org/jdk/pull/29158#discussion_r2699085620