JDK-8252890 : Obsolete InsertMemBarAfterArraycopy flag
  • Type: CSR
  • Component: hotspot
  • Sub-Component: compiler
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 16
  • Submitted: 2020-09-08
  • Updated: 2020-09-09
  • Resolved: 2020-09-09
Related Reports
CSR :  
Description
Summary
-------

Obsolete the broken -XX:-InsertMemBarAfterArraycopy C2 flag.

Problem
-------

The flag is enabled by default. Disabling the flag causes crashes during C2 compilation or generation of invalid code.

Solution
--------

The InsertMemBarAfterArraycopy flag will be obsoleted, meaning it will still be recognized but a warning will be generated at startup if it is specified on the command line.

Specification
-------------

Mark the flag as obsolete:

    diff -r bdc20ee1a68d src/hotspot/share/runtime/arguments.cpp
    --- a/src/hotspot/share/runtime/arguments.cpp   Fri Sep 04 23:51:26 2020 -0400
    +++ b/src/hotspot/share/runtime/arguments.cpp   Tue Sep 08 08:15:08 2020 +0200
    @@ -553,6 +553,7 @@
       { "UseNewFieldLayout",             JDK_Version::jdk(15), JDK_Version::jdk(16), JDK_Version::jdk(17) },
       { "UseSemaphoreGCThreadsSynchronization", JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
       { "ForceNUMA",                     JDK_Version::jdk(15), JDK_Version::jdk(16), JDK_Version::jdk(17) },
    +  { "InsertMemBarAfterArraycopy",    JDK_Version::undefined(), JDK_Version::jdk(16), JDK_Version::jdk(17) },
     
     #ifdef TEST_VERIFY_SPECIAL_JVM_FLAGS
       // These entries will generate build errors.  Their purpose is to test the macros.

Remove it from C2 code:

    diff -r bdc20ee1a68d src/hotspot/share/opto/c2_globals.hpp
    --- a/src/hotspot/share/opto/c2_globals.hpp     Fri Sep 04 23:51:26 2020 -0400
    +++ b/src/hotspot/share/opto/c2_globals.hpp     Tue Sep 08 08:15:08 2020 +0200
    @@ -384,9 +384,6 @@
       product(bool, UseOnlyInlinedBimorphic, true,                              \
               "Don't use BimorphicInlining if can't inline a second method")    \
                                                                                 \
    -  product(bool, InsertMemBarAfterArraycopy, true,                           \
    -          "Insert memory barrier after arraycopy call")                     \
    -                                                                            \
       develop(bool, SubsumeLoads, true,                                         \
               "Attempt to compile while subsuming loads into machine "          \
               "instructions.")                                                  \
Comments
Moving to Approved.
09-09-2020

Thanks David, I've changed the specification section accordingly.
08-09-2020

Immediate obsoletion, rather than deprecation does seem justified in this case. Note: there is no need to show the detailed code changes related to the flag removal in the "specification" section. The flag table entry in arguments.cpp and the change to globals.cpp, typically suffice for the "specification" of a flag change.
08-09-2020

Thanks, Nils!
08-09-2020

I agree - this option has no known uses, doesn't work, and isn't tested. Reviewed.
08-09-2020