[GitHub] [tvm] areusch commented on a diff in pull request #12440: [LLVM] Add "cl-opt" attribute to target_kind "llvm"

[GitBox <gi...@apache.org>]

areusch commented on code in PR #12440:
URL: https://github.com/apache/tvm/pull/12440#discussion_r949647671


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -153,6 +176,21 @@ LLVMTarget::LLVMTarget(LLVMInstance& instance, const Target& target)
     }
   }
 
+  if (const Optional<Array<String>>& v = target->GetAttr<Array<String>>("cl-opt")) {
+    llvm::StringMap<llvm::cl::Option*>& options = llvm::cl::getRegisteredOptions();
+    for (const String& s : v.value()) {
+      Option opt = ParseOptionString(s);
+      if (opt.type == Option::OptType::Invalid) {
+        continue;
+      }
+      if (options.count(opt.name)) {
+        llvm_options_.push_back(opt);
+      } else {
+        LOG(WARNING) << "\"" << opt.name << "\" is not an LLVM option, option ignored";

Review Comment:
   should we error instead? same question above on line 184



##########
src/target/llvm/llvm_instance.cc:
##########
@@ -340,9 +373,327 @@ std::string LLVMTarget::str() const {
     }
   }
 
+  if (size_t num = llvm_options_.size(); num > 0) {
+    os << " -cl-opt=";
+    std::vector<std::string> opts;
+    for (const Option& opt : llvm_options_) {
+      std::stringstream os;
+      os << opt;
+      opts.emplace_back(os.str());
+    }
+    auto* quote = num > 1 ? "'" : "";
+    os << quote << Join(",", opts) << quote;
+  }
+
   return os.str();
 }
 
+LLVMTargetInfo::Option LLVMTargetInfo::ParseOptionString(const std::string& str) {
+  Option opt;
+  opt.type = Option::OptType::Invalid;
+
+  // Option string: "-"+ <option_name> ":" <type> "=" <value>
+  //
+  // Note: "-"+ means 1 or more dashes, but only "-" are "--" valid.
+
+  // The first step is to do "lexing" of the option string, i.e. to break
+  // it up into parts (like "tokens") according to the syntax above. These
+  // parts will be non-overlapping substrings of the option string, and
+  // concatenated together, they will be equal to the option string.
+  // The literal elements are parts on their own.
+  //
+  // Note that the option string may be malformed, so any of the literal
+  // elements in the syntax may be missing.
+
+  std::vector<std::string> parts;
+
+  auto find_first_of = [](const std::string& str, const std::string& chars, auto start = 0) {
+    auto pos = str.find_first_of(chars, start);
+    return pos != std::string::npos ? pos : str.size();
+  };
+  auto find_first_not_of = [](const std::string& str, const std::string& chars, auto start = 0) {
+    auto pos = str.find_first_not_of(chars, start);
+    return pos != std::string::npos ? pos : str.size();
+  };
+
+  // "-"+
+  std::string::size_type pos_start = 0, pos_end = str.size();
+  std::string::size_type pos_at = find_first_not_of(str, "-", pos_start);
+  if (pos_at > 0) {
+    parts.push_back(str.substr(pos_start, pos_at));
+  }
+  // <option_name>, always present, may be empty string
+  pos_start = pos_at;
+  pos_at = find_first_of(str, ":=", pos_start);
+  parts.push_back(str.substr(pos_start, pos_at - pos_start));
+
+  // ":" or "=", if any
+  pos_start = pos_at;
+  char c = pos_start < pos_end ? str[pos_start] : 0;
+  if (c != 0) {
+    parts.emplace_back(1, c);
+    pos_start++;
+  }
+  // If the character found in the previous step wasn't '=', look for '='.
+  if (c == ':') {
+    // <type>
+    pos_at = find_first_of(str, "=", pos_start);
+    if (pos_at > pos_start) {  // if non-empty
+      parts.push_back(str.substr(pos_start, pos_at - pos_start));
+    }
+
+    // "="
+    if (pos_at < pos_end) {
+      parts.emplace_back(1, str[pos_at]);
+      pos_start = pos_at + 1;
+    }
+  }
+  if (pos_start < pos_end) {
+    // <value>
+    parts.push_back(str.substr(pos_start));
+  }
+
+  // After breaking up the option string, examine and validate the individual
+  // parts.
+
+  int part_this = 0, part_end = parts.size();
+
+  const std::string warn_header = "while parsing option \"" + str + "\": ";
+  const std::string warn_ignored = ", option ignored";
+
+  // Check for "-" or "--".
+  if (part_this < part_end) {
+    auto& p = parts[part_this++];
+    if ((p.size() != 1 && p.size() != 2) || p.find_first_not_of('-') != std::string::npos) {
+      LOG(WARNING) << warn_header << "option must start with \"-\" or \"--\"" << warn_ignored;
+      return opt;
+    }
+  }
+
+  // Validate option name.
+  if (part_this < part_end) {
+    auto& p = parts[part_this++];
+    if (p.empty()) {
+      LOG(WARNING) << warn_header << "option name must not be empty" << warn_ignored;
+      return opt;
+    }
+    opt.name = std::move(p);
+  }
+
+  // Check type, if present.
+  Option::OptType type = Option::OptType::Invalid;
+  if (part_this < part_end) {
+    auto& p0 = parts[part_this];
+    if (p0 == ":") {
+      part_this++;  // Only advance if we saw ":".
+      if (part_this < part_end) {
+        auto& p1 = parts[part_this];
+        ICHECK(!p1.empty()) << "tokenizing error";  // This shouldn't happen.
+        if (p1 != "=") {
+          part_this++;
+          if (p1 == "bool") {
+            type = Option::OptType::Bool;
+          } else if (p1 == "int") {
+            type = Option::OptType::Int;
+          } else if (p1 == "uint") {
+            type = Option::OptType::UInt;
+          } else if (p1 == "string") {
+            type = Option::OptType::String;
+          }
+        }
+      }
+      // If there was ":", there must be a type.
+      if (type == Option::OptType::Invalid) {
+        LOG(WARNING) << warn_header << "invalid type" << warn_ignored;
+        return opt;
+      }
+    }
+  }
+
+  // Check value, if present.
+  std::optional<std::string> value;
+  if (part_this < part_end) {
+    auto& p0 = parts[part_this];
+    if (p0 == "=") {
+      part_this++;
+      if (part_this < part_end) {
+        value = std::move(parts[part_this]);
+      } else {
+        value = "";
+      }
+    } else {
+      // If there are still any parts left to be processed, there must be "=".
+      LOG(WARNING) << warn_header << "expecting \"=\"" << warn_ignored;
+      return opt;
+    }
+  }
+
+  // NOLINTNEXTLINE(runtime/int)
+  auto to_integer = [](const std::string& s) -> std::optional<long long> {
+    // std::stoll takes "long long"
+    long long number;  // NOLINT(runtime/int)
+    size_t pos;
+    try {
+      number = std::stoll(s, &pos);
+    } catch (...) {
+      return std::nullopt;
+    }
+    if (pos == s.size()) {
+      return number;
+    } else {
+      return std::nullopt;
+    }
+  };
+
+  auto to_boolean = [&to_integer](const std::string& s) -> std::optional<bool> {
+    // Return 0 or 1, if string corresponds to a valid boolean value,
+    // otherwise return 2.
+    auto ti = to_integer(s);
+    if (ti.has_value() && (ti.value() == 0 || ti.value() == 1)) {
+      return static_cast<bool>(ti.value());
+    }
+
+    std::string lower;
+    std::transform(s.begin(), s.end(), std::back_inserter(lower),
+                   [](unsigned char c) { return std::tolower(c); });

Review Comment:
   could this just be std::tolower?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org