8211034: OnStackReplacePercentage option checking has bugs

Fixed the constraint checks

Reviewed-by: kvn

src/hotspot/share/interpreter/invocationCounter.cpp

@@ -153,9 +153,9 @@ void InvocationCounter::reinitialize(bool delay_overflow) {
   // don't need the shift by number_of_noncount_bits, but we do need to adjust
   // the factor by which we scale the threshold.
   if (ProfileInterpreter) {
-    InterpreterBackwardBranchLimit = (CompileThreshold * (OnStackReplacePercentage - InterpreterProfilePercentage)) / 100;
+    InterpreterBackwardBranchLimit = (int)((int64_t)CompileThreshold * (OnStackReplacePercentage - InterpreterProfilePercentage) / 100);
   } else {
-    InterpreterBackwardBranchLimit = ((CompileThreshold * OnStackReplacePercentage) / 100) << number_of_noncount_bits;
+    InterpreterBackwardBranchLimit = (int)(((int64_t)CompileThreshold * OnStackReplacePercentage / 100) << number_of_noncount_bits);
   }
 
   assert(0 <= InterpreterBackwardBranchLimit,

src/hotspot/share/oops/methodCounters.hpp

@@ -103,9 +103,9 @@ class MethodCounters : public Metadata {
       // If interpreter profiling is enabled, the backward branch limit
       // is compared against the method data counter rather than an invocation
       // counter, therefore no shifting of bits is required.
-      _interpreter_backward_branch_limit = (compile_threshold * (OnStackReplacePercentage - InterpreterProfilePercentage)) / 100;
+      _interpreter_backward_branch_limit = (int)((int64_t)compile_threshold * (OnStackReplacePercentage - InterpreterProfilePercentage) / 100);
     } else {
-      _interpreter_backward_branch_limit = ((compile_threshold * OnStackReplacePercentage) / 100) << InvocationCounter::count_shift;
+      _interpreter_backward_branch_limit = (int)(((int64_t)compile_threshold * OnStackReplacePercentage / 100) << InvocationCounter::count_shift);
     }
     _interpreter_profile_limit = ((compile_threshold * InterpreterProfilePercentage) / 100) << InvocationCounter::count_shift;
     _invoke_mask = right_n_bits(CompilerConfig::scaled_freq_log(Tier0InvokeNotifyFreqLog, scale)) << InvocationCounter::count_shift;

src/hotspot/share/opto/parseHelper.cpp

@@ -471,15 +471,15 @@ void Parse::profile_taken_branch(int target_bci, bool force_update) {
     if (osr_site) {
       ciProfileData* data = md->bci_to_data(cur_bci);
       assert(data != NULL && data->is_JumpData(), "need JumpData for taken branch");
-      int limit = (CompileThreshold
+      int limit = (int)((int64_t)CompileThreshold
-                   * (OnStackReplacePercentage - InterpreterProfilePercentage)) / 100;
+                   * (OnStackReplacePercentage - InterpreterProfilePercentage) / 100);
       test_for_osr_md_counter_at(md, data, JumpData::taken_offset(), limit);
     }
   } else {
     // With method data update off, use the invocation counter to trigger an
     // OSR compilation, as done in the interpreter.
     if (osr_site) {
-      int limit = (CompileThreshold * OnStackReplacePercentage) / 100;
+      int limit = (int)((int64_t)CompileThreshold * OnStackReplacePercentage / 100);
       increment_and_test_invocation_counter(limit);
     }
   }

src/hotspot/share/runtime/flags/jvmFlagConstraintsCompiler.cpp

@@ -145,47 +145,42 @@ JVMFlag::Error CompileThresholdConstraintFunc(intx value, bool verbose) {
 }
 
 JVMFlag::Error OnStackReplacePercentageConstraintFunc(intx value, bool verbose) {
-  int backward_branch_limit;
+  int64_t  max_percentage_limit = INT_MAX;
+  if (!ProfileInterpreter) {
+    max_percentage_limit = (max_percentage_limit>>InvocationCounter::count_shift);
+  }
+  max_percentage_limit = CompileThreshold == 0  ? max_percentage_limit*100 : max_percentage_limit*100/CompileThreshold;
+
   if (ProfileInterpreter) {
-    if (OnStackReplacePercentage < InterpreterProfilePercentage) {
+    if (value < InterpreterProfilePercentage) {
       JVMFlag::printError(verbose,
                           "OnStackReplacePercentage (" INTX_FORMAT ") must be "
                           "larger than InterpreterProfilePercentage (" INTX_FORMAT ")\n",
-                          OnStackReplacePercentage, InterpreterProfilePercentage);
+                          value, InterpreterProfilePercentage);
       return JVMFlag::VIOLATES_CONSTRAINT;
     }
 
-    backward_branch_limit = ((CompileThreshold * (OnStackReplacePercentage - InterpreterProfilePercentage)) / 100)
+    max_percentage_limit += InterpreterProfilePercentage;
-                            << InvocationCounter::count_shift;
+    if (value > max_percentage_limit) {
-
-    if (backward_branch_limit < 0) {
       JVMFlag::printError(verbose,
-                          "CompileThreshold * (InterpreterProfilePercentage - OnStackReplacePercentage) / 100 = "
+                          "OnStackReplacePercentage (" INTX_FORMAT ") must be between 0 and " INT64_FORMAT "\n",
-                          INTX_FORMAT " "
+                          value,
-                          "must be between 0 and %d, try changing "
+                          max_percentage_limit);
-                          "CompileThreshold, InterpreterProfilePercentage, and/or OnStackReplacePercentage\n",
-                          (CompileThreshold * (OnStackReplacePercentage - InterpreterProfilePercentage)) / 100,
-                          INT_MAX >> InvocationCounter::count_shift);
       return JVMFlag::VIOLATES_CONSTRAINT;
     }
   } else {
-    if (OnStackReplacePercentage < 0 ) {
+    if (value < 0) {
       JVMFlag::printError(verbose,
                           "OnStackReplacePercentage (" INTX_FORMAT ") must be "
-                          "non-negative\n", OnStackReplacePercentage);
+                          "non-negative\n", value);
       return JVMFlag::VIOLATES_CONSTRAINT;
     }
 
-    backward_branch_limit = ((CompileThreshold * OnStackReplacePercentage) / 100)
+    if (value > max_percentage_limit) {
-                            << InvocationCounter::count_shift;
-
-    if (backward_branch_limit < 0) {
       JVMFlag::printError(verbose,
-                          "CompileThreshold * OnStackReplacePercentage / 100 = " INTX_FORMAT " "
+                          "OnStackReplacePercentage (" INTX_FORMAT ") must be between 0 and " INT64_FORMAT "\n",
-                          "must be between 0 and %d, try changing "
+                          value,
-                          "CompileThreshold and/or OnStackReplacePercentage\n",
+                          max_percentage_limit);
-                          (CompileThreshold * OnStackReplacePercentage) / 100,
-                          INT_MAX >> InvocationCounter::count_shift);
       return JVMFlag::VIOLATES_CONSTRAINT;
     }
   }

test/hotspot/jtreg/testlibrary_tests/whitebox/vm_flags/IntxTest.java

@@ -34,17 +34,28 @@
  * @summary testing of WB::set/getIntxVMFlag()
  * @author igor.ignatyev@oracle.com
  */
-
+import jdk.test.lib.Platform;
 public class IntxTest {
     private static final String FLAG_NAME = "OnStackReplacePercentage";
     private static final String FLAG_DEBUG_NAME = "InlineFrequencyCount";
-    private static final Long[] TESTS = {0L, 100L, (long) Integer.MAX_VALUE};
+    private static final long COMPILE_THRESHOLD = VmFlagTest.WHITE_BOX.getIntxVMFlag("CompileThreshold");
+    private static final Long[] TESTS = {0L, 100L, (long)(Integer.MAX_VALUE>>3)*100L};
 
     public static void main(String[] args) throws Exception {
+        find_and_set_max_osrp();
         VmFlagTest.runTest(FLAG_NAME, TESTS,
             VmFlagTest.WHITE_BOX::setIntxVMFlag,
             VmFlagTest.WHITE_BOX::getIntxVMFlag);
         VmFlagTest.runTest(FLAG_DEBUG_NAME, VmFlagTest.WHITE_BOX::getIntxVMFlag);
     }
+
+    static void find_and_set_max_osrp() {
+        long max_percentage_limit = (long)(Integer.MAX_VALUE>>3)*100L;
+        max_percentage_limit = COMPILE_THRESHOLD == 0  ? max_percentage_limit : max_percentage_limit/COMPILE_THRESHOLD;
+        if (Platform.is32bit() && max_percentage_limit > Integer.MAX_VALUE) {
+          max_percentage_limit = Integer.MAX_VALUE;
+        }
+        TESTS[2] = max_percentage_limit;
+    }
 }