RFR: 8343148: C2: Refactor uses of "PhaseValue::*con*() + PhaseIdealLoop::set_ctrl()" into separate method [v3]

[Vladimir Kozlov]

On Thu, 7 Nov 2024 08:09:55 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

> > Do we have other places (not new constant node) where we set Root as control? May be we can add `set_root_as_ctrl(n)` method in `loop node.hpp` in such case.
> 
> There's only three locations where control is set to the root in the loop files now (not counting the ones in the new methods I added). The main reason for this patch is bugs caused by people forgetting to set control for constants (e.g. https://bugs.openjdk.org/browse/JDK-8343137), which is now prevented if the new helper methods are used.
> 
> Do you think there would be any benefit from introducing `set_root_as_ctrl(n)` given there's only about three places where this pattern occurs now?

My suggesting is about additional cleaning code. I think 3 + 5 places are enough to justify to have a new function in header file. Also `set_root_as_ctrl(n)` could be copy of `set_ctrl(n, ctrl)` without 2 asserts which checks `ctrl`. It will be faster.

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

PR Comment: https://git.openjdk.org/jdk/pull/21836#issuecomment-2462873767