JDK-8317419 : Obsolete DoReserveCopyInSuperWord
  • Type: CSR
  • Component: hotspot
  • Sub-Component: compiler
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 22
  • Submitted: 2023-10-03
  • Updated: 2023-10-05
  • Resolved: 2023-10-04
Related Reports
CSR :  
Description
Summary
-------

In JDK-8309204, we would like to remove all functionality of DoReserveCopyInSuperWord, after which this product flag becomes useless.

I already have a [PR for JDK-8309204][1].

Problem
-------

Currently, the flag DoReserveCopyInSuperWord (on by default) creates a copy of the loop at the beginning of SuperWord::output, so that if anything goes wrong while modifying the graph, we can revert to a unmodified copy. If a user explicitly disables the flag, we would instead hit a ShouldNotReachHere and crash.

But these are absolute edge cases: only when we hit asserts in debug mode. Hence, it is not justified to have the extra computational overhead of cloning/copying the loop, and the code complexity of CountedLoopReserveKit.

Solution
-------

Instead of copy / revert, we simply bail out and recompile without SuperWord.

Remove CountedLoopReserveKit.

Obsolete DoReserveCopyInSuperWord. Simply deprecating it would not allow for the removal of the functionality, hence we want to go for obsoletion directly.

Specification
-------

The changes to argument handling:

    --- a/src/hotspot/share/opto/c2_globals.hpp
    +++ b/src/hotspot/share/opto/c2_globals.hpp
    @@ -341,9 +341,6 @@
       product(bool, UseCMoveUnconditionally, false,                             \
               "Use CMove (scalar and vector) ignoring profitability test.")     \
                                                                                 \
    -  product(bool, DoReserveCopyInSuperWord, true,                             \
    -          "Create reserve copy of graph in SuperWord.")                     \
    -                                                                            \
       notproduct(bool, TraceSuperWord, false,                                   \
               "Trace superword transforms")                                     \
                                                                                 \

    --- a/src/hotspot/share/runtime/arguments.cpp
    +++ b/src/hotspot/share/runtime/arguments.cpp
    @@ -524,6 +524,7 @@ static SpecialFlag const special_jvm_flags[] = {
       { "G1ConcRSHotCardLimit",         JDK_Version::undefined(), JDK_Version::jdk(21), JDK_Version::undefined() },
       { "RefDiscoveryPolicy",           JDK_Version::undefined(), JDK_Version::jdk(21), JDK_Version::undefined() },
       { "MetaspaceReclaimPolicy",       JDK_Version::undefined(), JDK_Version::jdk(21), JDK_Version::undefined() },
    +  { "DoReserveCopyInSuperWord",     JDK_Version::undefined(), JDK_Version::jdk(22), JDK_Version::jdk(23) },
     
     #ifdef ASSERT
       { "DummyObsoleteTestFlag",        JDK_Version::undefined(), JDK_Version::jdk(18), JDK_Version::undefined() },

Further, all uses of DoReserveCopyInSuperWord are removed.

  [1]: https://github.com/openjdk/jdk/pull/16022
Comments
`DoReserveCopyInSuperWord` was internal C2 flag not for Java users. I agree that we don't need release note for this.
05-10-2023

[~darcy] thanks for the approval! I discussed it with [~thartmann]. We don't think a release note is required. [~kvn] let us know if you disagree.
05-10-2023

Moving updated request to Approved. [~epeter], I'll let it to the HotSpot team's discretion as to whether or not this change merits a release note.
04-10-2023

[~darcy] I added the diff for the two files that handle the argument.
04-10-2023

Moving to Provisional, not Approved. A link to a PR is not suitable to provide the Specification portion of a CSR. In CSRs similar to this, engineers have included the diff of the particular file that control the argument handling.
03-10-2023

Thanks [~thartmann], moving it to "Finalized" and waiting on CSR team to review/approve it.
03-10-2023

We would like to obsolete this without former deprecation because the flag is essentially useless/broken as explained in the description.
03-10-2023