8257460: Further CompilerOracle cleanup

Reviewed-by: kvn, redestad, thartmann

Author: Nils Eliasson

src/hotspot/share/compiler/compilerDirectives.cpp

@@ -359,7 +359,7 @@ DirectiveSet* DirectiveSet::compilecommand_compatibility_init(const methodHandle
     }
 
     // inline and dontinline (including exclude) are implemented in the directiveset accessors
-#define init_default_cc(name, type, dvalue, cc_flag) { type v; if (!_modified[name##Index] && CompilerOracle::has_option_value(method, CompileCommand::cc_flag, v) && v != this->name##Option) { set.cloned()->name##Option = v; } }
+#define init_default_cc(name, type, dvalue, cc_flag) { type v; if (!_modified[name##Index] && CompileCommand::cc_flag != CompileCommand::Unknown && CompilerOracle::has_option_value(method, CompileCommand::cc_flag, v) && v != this->name##Option) { set.cloned()->name##Option = v; } }
     compilerdirectives_common_flags(init_default_cc)
     compilerdirectives_c2_flags(init_default_cc)
     compilerdirectives_c1_flags(init_default_cc)

src/hotspot/share/compiler/compilerOracle.cpp

@@ -104,7 +104,6 @@ class TypedMethodOptionMatcher : public MethodMatcher {
  private:
   TypedMethodOptionMatcher* _next;
   enum CompileCommand _option;
-  OptionType    _type;
  public:
 
   union {
@@ -117,18 +116,16 @@ class TypedMethodOptionMatcher : public MethodMatcher {
 
   TypedMethodOptionMatcher() : MethodMatcher(),
     _next(NULL),
-    _option(CompileCommand::Unknown),
-    _type(OptionType::Unknown) {
+    _option(CompileCommand::Unknown) {
       memset(&_u, 0, sizeof(_u));
   }
 
   ~TypedMethodOptionMatcher();
   static TypedMethodOptionMatcher* parse_method_pattern(char*& line, char* errorbuf, const int buf_size);
-  TypedMethodOptionMatcher* match(const methodHandle &method, enum CompileCommand option, OptionType type);
+  TypedMethodOptionMatcher* match(const methodHandle &method, enum CompileCommand option);
 
-  void init(enum CompileCommand option, OptionType type, TypedMethodOptionMatcher* next) {
+  void init(enum CompileCommand option, TypedMethodOptionMatcher* next) {
     _next = next;
-    _type = type;
     _option = option;
   }
 
@@ -140,7 +137,6 @@ class TypedMethodOptionMatcher : public MethodMatcher {
 
   void set_next(TypedMethodOptionMatcher* next) {_next = next; }
   TypedMethodOptionMatcher* next() { return _next; }
-  OptionType type() { return _type; }
   enum CompileCommand option() { return _option; }
   template<typename T> T value();
   template<typename T> void set_value(T value);
@@ -194,8 +190,9 @@ void TypedMethodOptionMatcher::print() {
   ttyLocker ttyl;
   print_base(tty);
   const char* name = option2name(_option);
-  switch (_type) {
-  case OptionType::Intx:
+  enum OptionType type = option2type(_option);
+  switch (type) {
+    case OptionType::Intx:
     tty->print_cr(" intx %s = " INTX_FORMAT, name, value<intx>());
     break;
     case OptionType::Uintx:
@@ -208,6 +205,7 @@ void TypedMethodOptionMatcher::print() {
     tty->print_cr(" double %s = %f", name, value<double>());
     break;
     case OptionType::Ccstr:
+    case OptionType::Ccstrlist:
     tty->print_cr(" const char* %s = '%s'", name, value<ccstr>());
     break;
   default:
@@ -244,7 +242,8 @@ TypedMethodOptionMatcher* TypedMethodOptionMatcher::clone() {
 }
 
 TypedMethodOptionMatcher::~TypedMethodOptionMatcher() {
-  if (type() == OptionType::Ccstr) {
+  enum OptionType type = option2type(_option);
+  if (type == OptionType::Ccstr || type == OptionType::Ccstrlist) {
     ccstr v = value<ccstr>();
     os::free((void*)v);
   }
@@ -263,7 +262,7 @@ TypedMethodOptionMatcher* TypedMethodOptionMatcher::parse_method_pattern(char*&
   return tom;
 }
 
-TypedMethodOptionMatcher* TypedMethodOptionMatcher::match(const methodHandle& method, enum CompileCommand option, OptionType type) {
+TypedMethodOptionMatcher* TypedMethodOptionMatcher::match(const methodHandle& method, enum CompileCommand option) {
   TypedMethodOptionMatcher* current = this;
   while (current != NULL) {
     if (current->_option == option) {
@@ -285,12 +284,9 @@ static void register_command(TypedMethodOptionMatcher* matcher,
     tty->print_cr("Warning:  +LogCompilation must be enabled in order for individual methods to be logged with ");
     tty->print_cr("          CompileCommand=log,<method pattern>");
   }
-  enum OptionType type = option2type(option);
-  if (type == OptionType::Ccstrlist) {
-    type = OptionType::Ccstr; // ccstrlists are stores as ccstr
-  }
-  assert(type == get_type_for<T>(), "sanity");
-  matcher->init(option, type, option_list);
+  assert(CompilerOracle::option_matches_type(option, value), "Value must match option type");
+
+  matcher->init(option, option_list);
   matcher->set_value<T>(value);
   option_list = matcher;
   if ((option != CompileCommand::DontInline) &&
@@ -308,26 +304,10 @@ static void register_command(TypedMethodOptionMatcher* matcher,
 }
 
 template<typename T>
-bool CompilerOracle::has_option_value(const methodHandle& method, enum CompileCommand option, T& value, bool verify_type) {
-  enum OptionType type = option2type(option);
-  if (type == OptionType::Unknown) {
-    return false; // Can't query options with type Unknown.
-  }
-  if (type == OptionType::Ccstrlist) {
-    type = OptionType::Ccstr; // CCstrList type options are stored as Ccstr
-  }
-  if (verify_type) {
-    if (type != get_type_for<T>()) {
-      // Whitebox API expects false if option and type doesn't match
-      return false;
-    }
-  } else {
-    assert(type == get_type_for<T>(), "Value type (%s) must match option %s (%s)",
-            optiontype2name(get_type_for<T>()),
-           option2name(option), optiontype2name(option2type(option)));
-  }
+bool CompilerOracle::has_option_value(const methodHandle& method, enum CompileCommand option, T& value) {
+  assert(option_matches_type(option, value), "Value must match option type");
   if (option_list != NULL) {
-    TypedMethodOptionMatcher* m = option_list->match(method, option, type);
+    TypedMethodOptionMatcher* m = option_list->match(method, option);
     if (m != NULL) {
       value = m->value<T>();
       return true;
@@ -361,11 +341,29 @@ bool CompilerOracle::has_any_command_set() {
 }
 
 // Explicit instantiation for all OptionTypes supported.
-template bool CompilerOracle::has_option_value<intx>(const methodHandle& method, enum CompileCommand option, intx& value, bool verify_type);
-template bool CompilerOracle::has_option_value<uintx>(const methodHandle& method, enum CompileCommand option, uintx& value, bool verify_type);
-template bool CompilerOracle::has_option_value<bool>(const methodHandle& method, enum CompileCommand option, bool& value, bool verify_type);
-template bool CompilerOracle::has_option_value<ccstr>(const methodHandle& method, enum CompileCommand option, ccstr& value, bool verify_type);
-template bool CompilerOracle::has_option_value<double>(const methodHandle& method, enum CompileCommand option, double& value, bool verify_type);
+template bool CompilerOracle::has_option_value<intx>(const methodHandle& method, enum CompileCommand option, intx& value);
+template bool CompilerOracle::has_option_value<uintx>(const methodHandle& method, enum CompileCommand option, uintx& value);
+template bool CompilerOracle::has_option_value<bool>(const methodHandle& method, enum CompileCommand option, bool& value);
+template bool CompilerOracle::has_option_value<ccstr>(const methodHandle& method, enum CompileCommand option, ccstr& value);
+template bool CompilerOracle::has_option_value<double>(const methodHandle& method, enum CompileCommand option, double& value);
+
+template<typename T>
+bool CompilerOracle::option_matches_type(enum CompileCommand option, T& value) {
+  enum OptionType option_type = option2type(option);
+  if (option_type == OptionType::Unknown) {
+    return false; // Can't query options with type Unknown.
+  }
+  if (option_type == OptionType::Ccstrlist) {
+    option_type = OptionType::Ccstr; // CCstrList type options are stored as Ccstr
+  }
+  return (get_type_for<T>() == option_type);
+}
+
+template bool CompilerOracle::option_matches_type<intx>(enum CompileCommand option, intx& value);
+template bool CompilerOracle::option_matches_type<uintx>(enum CompileCommand option, uintx& value);
+template bool CompilerOracle::option_matches_type<bool>(enum CompileCommand option, bool& value);
+template bool CompilerOracle::option_matches_type<ccstr>(enum CompileCommand option, ccstr& value);
+template bool CompilerOracle::option_matches_type<double>(enum CompileCommand option, double& value);
 
 bool CompilerOracle::has_option(const methodHandle& method, enum CompileCommand option) {
   bool value = false;

src/hotspot/share/compiler/compilerOracle.hpp

@@ -149,7 +149,11 @@ class CompilerOracle : AllStatic {
   // Check if method has option and value set. If yes, overwrite value and return true,
   // otherwise leave value unchanged and return false.
   template<typename T>
-  static bool has_option_value(const methodHandle& method, enum CompileCommand option, T& value, bool verfiy_type = false);
+  static bool has_option_value(const methodHandle& method, enum CompileCommand option, T& value);
+
+  // This check is currently only needed by whitebox API
+  template<typename T>
+  static bool option_matches_type(enum CompileCommand option, T& value);
 
   // Reads from string instead of file
   static void parse_from_string(const char* option_string, void (*parser)(char*));

src/hotspot/share/prims/whitebox.cpp

@@ -1814,7 +1814,10 @@ static bool GetMethodOption(JavaThread* thread, JNIEnv* env, jobject method, jst
   if (option == CompileCommand::Unknown) {
     return false;
   }
-  return CompilerOracle::has_option_value(mh, option, *value, true /* verify type*/);
+  if (!CompilerOracle::option_matches_type(option, *value)) {
+    return false;
+  }
+  return CompilerOracle::has_option_value(mh, option, *value);
 }
 
 WB_ENTRY(jobject, WB_GetMethodBooleaneOption(JNIEnv* env, jobject wb, jobject method, jstring name))