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

[Roland Westrelin]

On Thu, 22 Jan 2026 15:23:55 GMT, Roland Westrelin <roland 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
>
> As I understand, this change removes logic that's overly conservative but doesn't address any correctness issue (i.e. there's no crash or incorrect execution that this fixes). Given the new logic is less conservative, there should be cases where the code optimizes better with this change. Would it make sense to add IR test cases to catch regressions?

> @rwestrel Thanks for taking a look, this PR makes it strictly more conservative by pinning more nodes when an `IfNode` is elided. However, this conservativeness is necessary, any optimization that can arise from a node not pinned is incorrect, such as [JDK-8331717](https://bugs.openjdk.org/browse/JDK-8331717) or [JDK-8257822](https://bugs.openjdk.org/browse/JDK-8257822).

But those bugs are fixed. Are there known bugs that this PR fixes?
The way I understand it, you're replacing some existing logic that addresses this kind of issues with some new logic that works some other way but addresses the same issues.
Also, with attached test case, current c2 code compiles the method with 2 `DivI` nodes while, with your patch, it can common the 2 identical `DivI` nodes and we end up with one. That feels less conservative. So I'm wondering in what way it is more conservative.

(FYI, I agree with the general direction of this patch)

[TestRedundantDivs.java](https://github.com/user-attachments/files/24824874/TestRedundantDivs.java)

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

PR Comment: https://git.openjdk.org/jdk/pull/29158#issuecomment-3791067096