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

[Tobias Hartmann]

On Wed, 7 May 2025 08:34:28 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>>> Is this test still valid?
>> 
>> I think it's as valid as any other regression test. With time, there's no guarantee that the test would still trigger the original issue if the fix would accidentally be reverted. But these tests still have a lot of value because they trigger state that apparently no other test triggered and often they still reproduce the issue with older JDK releases (or reveal new issues with new changes).
>> 
>>> but this assert appears to be removed?
>> 
>> It's still here, see:
>> https://github.com/openjdk/jdk/blob/910d77d39e6fb9ca339272c75fa4ff7ff99bffcf/src/hotspot/share/opto/gcm.cpp#L889-L890
>> 
>>> 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?
>> 
>> It's not possible to disable loop strip mining but IIRC I disabled it manually to see what other issues we hit without verification, for example in release builds (see also my comments in https://github.com/openjdk/jdk/pull/2315). So this test basically covers a different failure mode.
>> 
>> I think Daniel's change is good as it is.
>
> Thanks @TobiHartmann. Note that this changeset does remove the assert, and replaces it with another (stronger, in theory) assert.

Ah right, I missed that.

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

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