Merge "ART: Pass Options should be strings instead of integers"

3 files changed, 211 insertions, 20 deletions

diff --git a/compiler/dex/compiler_ir.h b/compiler/dex/compiler_ir.h
index 51c4a43..0c46d43 100644
--- a/compiler/dex/compiler_ir.h
+++ b/compiler/dex/compiler_ir.h
@@ -34,6 +34,129 @@ class CompilerDriver;
 class Mir2Lir;
 class MIRGraph;
 
+constexpr size_t kOptionStringMaxLength = 2048;
+
+/**
+ * Structure abstracting pass option values, which can be of type string or integer.
+ */
+struct OptionContent {
+  OptionContent(const OptionContent& option) :
+    type(option.type), container(option.container, option.type) {}
+
+  explicit OptionContent(const char* value) :
+    type(kString), container(value) {}
+
+  explicit OptionContent(int value) :
+    type(kInteger), container(value) {}
+
+  explicit OptionContent(int64_t value) :
+    type(kInteger), container(value) {}
+
+  ~OptionContent() {
+    if (type == kString) {
+      container.StringDelete();
+    }
+  }
+
+  /**
+   * Allows for a transparent display of the option content.
+   */
+  friend std::ostream& operator<<(std::ostream& out, const OptionContent& option) {
+    if (option.type == kString) {
+      out << option.container.s;
+    } else {
+      out << option.container.i;
+    }
+
+    return out;
+  }
+
+  inline const char* GetString() const {
+    return container.s;
+  }
+
+  inline int64_t GetInteger() const {
+    return container.i;
+  }
+
+  /**
+   * @brief Used to compare a string option value to a given @p value.
+   * @details Will return whether the internal string option is equal to
+   * the parameter @p value. It will return false if the type of the
+   * object is not a string.
+   * @param value The string to compare to.
+   * @return Returns whether the internal string option is equal to the
+   * parameter @p value.
+  */
+  inline bool Equals(const char* value) const {
+    DCHECK(value != nullptr);
+    if (type != kString) {
+      return false;
+    }
+    return !strncmp(container.s, value, kOptionStringMaxLength);
+  }
+
+  /**
+   * @brief Used to compare an integer option value to a given @p value.
+   * @details Will return whether the internal integer option is equal to
+   * the parameter @p value. It will return false if the type of the
+   * object is not an integer.
+   * @param value The integer to compare to.
+   * @return Returns whether the internal integer option is equal to the
+   * parameter @p value.
+  */
+  inline bool Equals(int64_t value) const {
+    if (type != kInteger) {
+      return false;
+    }
+    return container.i == value;
+  }
+
+  /**
+   * Describes the type of parameters allowed as option values.
+   */
+  enum OptionType {
+    kString = 0,
+    kInteger
+  };
+
+  OptionType type;
+
+ private:
+  /**
+   * Union containing the option value of either type.
+   */
+  union OptionContainer {
+    explicit OptionContainer(const OptionContainer& c, OptionType t) {
+      if (t == kString) {
+        DCHECK(c.s != nullptr);
+        s = strndup(c.s, kOptionStringMaxLength);
+      } else {
+        i = c.i;
+      }
+    }
+
+    explicit OptionContainer(const char* value) {
+      DCHECK(value != nullptr);
+      s = strndup(value, kOptionStringMaxLength);
+    }
+
+    explicit OptionContainer(int64_t value) : i(value) {}
+    ~OptionContainer() {}
+
+    void StringDelete() {
+      if (s != nullptr) {
+        free(s);
+      }
+    }
+
+    char* s;
+    int64_t i;
+  };
+
+  OptionContainer container;
+};
+
 struct CompilationUnit {
   CompilationUnit(ArenaPool* pool, InstructionSet isa, CompilerDriver* driver, ClassLinker* linker);
   ~CompilationUnit();
@@ -77,7 +200,7 @@ struct CompilationUnit {
    * default settings have been changed. The key is simply the option string without
    * the pass name.
    */
-  SafeMap<const std::string, int> overridden_pass_options;
+  SafeMap<const std::string, const OptionContent> overridden_pass_options;
 };
 
 }  // namespace art
diff --git a/compiler/dex/pass_driver_me.h b/compiler/dex/pass_driver_me.h
index fed92be..94eef22 100644
--- a/compiler/dex/pass_driver_me.h
+++ b/compiler/dex/pass_driver_me.h
@@ -165,7 +165,7 @@ class PassDriverME: public PassDriver {
       const PassME* me_pass = down_cast<const PassME*>(pass);
       if (me_pass->HasOptions()) {
         LOG(INFO) << "Pass options for \"" << me_pass->GetName() << "\" are:";
-        SafeMap<const std::string, int> overridden_settings;
+        SafeMap<const std::string, const OptionContent> overridden_settings;
         FillOverriddenPassSettings(&manager->GetOptions(), me_pass->GetName(),
                                    overridden_settings);
         me_pass->PrintPassOptions(overridden_settings);
@@ -212,7 +212,7 @@ class PassDriverME: public PassDriver {
    * configuration.
    */
   static void FillOverriddenPassSettings(const PassManagerOptions* options, const char* pass_name,
-                                         SafeMap<const std::string, int>& settings_to_fill) {
+                                         SafeMap<const std::string, const OptionContent>& settings_to_fill) {
     const std::string& settings = options->GetOverriddenPassOptions();
     const size_t settings_len = settings.size();
 
@@ -285,17 +285,28 @@ class PassDriverME: public PassDriver {
           continue;
       }
 
-      // Get the actual setting itself. Strtol is being used to convert because it is
-      // exception safe. If the input is not sane, it will set a setting of 0.
+      // Get the actual setting itself.
       std::string setting_string =
           settings.substr(setting_pos, next_configuration_separator - setting_pos);
-      int setting = std::strtol(setting_string.c_str(), 0, 0);
 
       std::string setting_name =
           settings.substr(setting_name_pos, setting_pos - setting_name_pos - 1);
 
-      settings_to_fill.Put(setting_name, setting);
-
+      // We attempt to convert the option value to integer. Strtoll is being used to
+      // convert because it is exception safe.
+      char* end_ptr = nullptr;
+      const char* setting_ptr = setting_string.c_str();
+      DCHECK(setting_ptr != nullptr);  // Paranoid: setting_ptr must be a valid pointer.
+      int64_t int_value = strtoll(setting_ptr, &end_ptr, 0);
+      DCHECK(end_ptr != nullptr);  // Paranoid: end_ptr must be set by the strtoll call.
+
+      // If strtoll call succeeded, the option is now considered as integer.
+      if (*setting_ptr != '\0' && end_ptr != setting_ptr && *end_ptr == '\0') {
+        settings_to_fill.Put(setting_name, OptionContent(int_value));
+      } else {
+        // Otherwise, it is considered as a string.
+        settings_to_fill.Put(setting_name, OptionContent(setting_string.c_str()));
+      }
       search_pos = next_configuration_separator;
     } while (true);
   }
diff --git a/compiler/dex/pass_me.h b/compiler/dex/pass_me.h
index 79d8f51..d3cf393 100644
--- a/compiler/dex/pass_me.h
+++ b/compiler/dex/pass_me.h
@@ -21,6 +21,7 @@
 
 #include "base/logging.h"
 #include "pass.h"
+#include "compiler_ir.h"
 #include "safe_map.h"
 
 namespace art {
@@ -104,7 +105,7 @@ class PassME : public Pass {
    */
   void PrintPassDefaultOptions() const {
     for (const auto& option : default_options_) {
-      LOG(INFO) << "\t" << option.first << ":" << std::dec << option.second;
+      LOG(INFO) << "\t" << option.first << ":" << option.second;
     }
   }
 
@@ -112,15 +113,49 @@ class PassME : public Pass {
    * @brief Prints the pass options along with either default or overridden setting.
    * @param overridden_options The overridden settings for this pass.
    */
-  void PrintPassOptions(SafeMap<const std::string, int>& overridden_options) const {
+  void PrintPassOptions(SafeMap<const std::string, const OptionContent>& overridden_options) const {
     // We walk through the default options only to get the pass names. We use GetPassOption to
     // also consider the overridden ones.
     for (const auto& option : default_options_) {
-      LOG(INFO) << "\t" << option.first << ":" << std::dec
+      LOG(INFO) << "\t" << option.first << ":"
                 << GetPassOption(option.first, overridden_options);
     }
   }
 
+  /**
+   * @brief Used to obtain the option structure for a pass.
+   * @details Will return the overridden option if it exists or default one otherwise.
+   * @param option_name The name of option whose setting to look for.
+   * @param c_unit The compilation unit currently being handled.
+   * @return Returns the option structure containing the option value.
+  */
+  const OptionContent& GetPassOption(const char* option_name, CompilationUnit* c_unit) const {
+    return GetPassOption(option_name, c_unit->overridden_pass_options);
+  }
+
+  /**
+   * @brief Used to obtain the option for a pass as a string.
+   * @details Will return the overridden option if it exists or default one otherwise.
+   * It will return nullptr if the required option value is not a string.
+   * @param option_name The name of option whose setting to look for.
+   * @param c_unit The compilation unit currently being handled.
+   * @return Returns the overridden option if it exists or the default one otherwise.
+  */
+  const char* GetStringPassOption(const char* option_name, CompilationUnit* c_unit) const {
+    return GetStringPassOption(option_name, c_unit->overridden_pass_options);
+  }
+
+  /**
+    * @brief Used to obtain the pass option value as an integer.
+    * @details Will return the overridden option if it exists or default one otherwise.
+    * It will return 0 if the required option value is not an integer.
+    * @param c_unit The compilation unit currently being handled.
+    * @return Returns the overriden option if it exists or the default one otherwise.
+   */
+  int64_t GetIntegerPassOption(const char* option_name, CompilationUnit* c_unit) const {
+    return GetIntegerPassOption(option_name, c_unit->overridden_pass_options);
+  }
+
   const char* GetDumpCFGFolder() const {
     return dump_cfg_folder_;
   }
@@ -130,29 +165,51 @@ class PassME : public Pass {
   }
 
  protected:
-  int GetPassOption(const char* option_name, const SafeMap<const std::string, int>& overridden_options) const {
+  const OptionContent& GetPassOption(const char* option_name,
+        const SafeMap<const std::string, const OptionContent>& overridden_options) const {
+    DCHECK(option_name != nullptr);
+
     // First check if there are any overridden settings.
     auto overridden_it = overridden_options.find(std::string(option_name));
     if (overridden_it != overridden_options.end()) {
       return overridden_it->second;
+    } else {
+      // Otherwise, there must be a default value for this option name.
+      auto default_it = default_options_.find(option_name);
+      // An invalid option is being requested.
+      if (default_it == default_options_.end()) {
+        LOG(FATAL) << "Fatal: Cannot find an option named \"" << option_name << "\"";
+      }
+
+      return default_it->second;
     }
+  }
 
-    // Next check the default options.
-    auto default_it = default_options_.find(option_name);
+  const char* GetStringPassOption(const char* option_name,
+        const SafeMap<const std::string, const OptionContent>& overridden_options) const {
+    const OptionContent& option_content = GetPassOption(option_name, overridden_options);
+    if (option_content.type != OptionContent::kString) {
+      return nullptr;
+    }
 
-    if (default_it == default_options_.end()) {
-      // An invalid option is being requested.
-      DCHECK(false);
+    return option_content.GetString();
+  }
+
+  int64_t GetIntegerPassOption(const char* option_name,
+          const SafeMap<const std::string, const OptionContent>& overridden_options) const {
+    const OptionContent& option_content = GetPassOption(option_name, overridden_options);
+    if (option_content.type != OptionContent::kInteger) {
       return 0;
     }
 
-    return default_it->second;
+    return option_content.GetInteger();
   }
 
   /** @brief Type of traversal: determines the order to execute the pass on the BasicBlocks. */
   const DataFlowAnalysisMode traversal_type_;
 
-  /** @brief Flags for additional directives: used to determine if a particular post-optimization pass is necessary. */
+  /** @brief Flags for additional directives: used to determine if a particular
+    * post-optimization pass is necessary. */
   const unsigned int flags_;
 
   /** @brief CFG Dump Folder: what sub-folder to use for dumping the CFGs post pass. */
@@ -163,7 +220,7 @@ class PassME : public Pass {
    * @details The constructor of the specific pass instance should fill this
    * with default options.
    * */
-  SafeMap<const char*, int> default_options_;
+  SafeMap<const char*, const OptionContent> default_options_;
 };
 }  // namespace art
 #endif  // ART_COMPILER_DEX_PASS_ME_H_