RFR: 8351568: Improve source code documentation for PhaseCFG::insert_anti_dependences [v2]

[Daniel Lundén]

On Tue, 6 May 2025 09:38:37 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> Unfortunately, it was not straightforward to revert the fix for [JDK-8260420](https://bugs.openjdk.org/browse/JDK-8260420) (too many changes since then). If `!LCA_orig->dominates(pred_block) || early->dominates(pred_block)` failed at some point, then the new assert `early->dominates(LCA_orig)` must also fail in that situation (in theory). See the details in my other response above.
>
> Ok, maybe I was not clear enough.
> 
> The comment says:
> 
>>     // Triggers an assert in PhaseCFG::raise_above_anti_dependences if loop strip mining verification is disabled:
> 
> My question is, does the test on top of which the comment is placed (`test4` right?) ever run with loop strip mining verification disabled and if it does, does the assert get triggered? If `test4` does not do this, seems to me it would be nice to have an additional test that verifies just that rather than accept/assume the comment as valid without a test that actually verifies this. Thoughts?

I would assume `test4` is a regression test and that the `assert` no longer triggers in any situation (otherwise we'd still have a bug)? Also, from what I can see, there is no VM flag that disables loop strip mining verification.

Perhaps I'm still misunderstanding you? @TobiHartmann I see you added this test back in 2021, could you help bring us some clarity? This changeset only renames the occurrence of `insert_anti_dependences` in `TestSplitIfPinnedLoadInStripMinedLoop.java` to `raise_above_anti_dependences`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24926#discussion_r2076003886