[clang] Fix -Xarch_ for -mllvm and alike

[yaxunl]

Summary

Currently -Xarch_ does not work for options using space as delimiter, e.g.
-mllvm, which is a big limitation for -Xarch_ as device specific llvm options
are often needed.

The main obstacle is in OptTable::ParseOneArg, which can only parse
one newly added string to the base InputArgList of a DerivedArgList,
since Option::acceptInternal is bounded by size of the original strings
in InputArgList. The reason is that the newly added strings are used
to create new args in derived arg list, which usually happens during
parsing of the original arg strings, therefore Option::acceptInternal
should not go beyond the original arg strings.

In the case of -Xarch handling, if the newly created arg have two
strings, e.g. "-mllvm" and "--inline-threshold-count=1",
OptTable::ParseOneArg needs to be enhanced to handle a
range of newly added strings beyond the original strings. A new
parameter IndexEnd is added.

[MaskRay]

An option value treating a space differently (as a delimiter in this case) is uncommon. I suggest an alias -mllvm= (D143325) which is more inline with the convention of the majority of options that take a value (= instead of ).

[MaskRay]

I assume that this is unneeded after :)

[yaxunl]

Abandon this patch unless we need it in the future.

clang/lib/Driver/Driver.cpp

Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args,
   llvm::DenseSet<StringRef> Archs;
   for (auto *Arg : Args) {
     // Extract any '--[no-]offload-arch' arguments intended for this toolchain.
     std::unique_ptr<llvm::opt::Arg> ExtractedArg = nullptr;
     if (Arg->getOption().matches(options::OPT_Xopenmp_target_EQ) &&
         ToolChain::getOpenMPTriple(Arg->getValue(0)) == TC->getTriple()) {
       Arg->claim();
       unsigned Index = Args.getBaseArgs().MakeIndex(Arg->getValue(1));
-      ExtractedArg = getOpts().ParseOneArg(Args, Index);
+      ExtractedArg =
+          getOpts().ParseOneArg(Args, Index, Args.getNumInputArgStrings());
       Arg = ExtractedArg.get();
     }
 
     // Add or remove the seen architectures in order of appearance. If an
     // invalid architecture is given we simply exit.
     if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
       for (StringRef Arch : llvm::split(Arg->getValue(), ",")) {
         if (Arch == "native" || Arch.empty()) {

clang/lib/Driver/ToolChain.cpp

 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <cassert>
 #include <cstddef>
 #include <cstring>
+#include <sstream>
 #include <string>
 
 using namespace clang;
 using namespace driver;
 using namespace tools;
 using namespace llvm;
 using namespace llvm::opt;
 
if (A->getOption().matches(options::OPT_Xopenmp_target_EQ)) {
       Index = Args.getBaseArgs().MakeIndex(A->getValue(0));
     } else {
       DAL->append(A);
       continue;
     }
 
     // Parse the argument to -Xopenmp-target.
     Prev = Index;
-    std::unique_ptr<Arg> XOpenMPTargetArg(Opts.ParseOneArg(Args, Index));
+    std::unique_ptr<Arg> XOpenMPTargetArg(
+        Opts.ParseOneArg(Args, Index, Args.getNumInputArgStrings()));
     if (!XOpenMPTargetArg || Index > Prev + 1) {
       getDriver().Diag(diag::err_drv_invalid_Xopenmp_target_with_args)
           << A->getAsString(Args);
       continue;
     }
     if (XOpenMPTargetNoTriple && XOpenMPTargetArg &&
         Args.getAllArgValues(options::OPT_fopenmp_targets_EQ).size() != 1) {
       getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
void ToolChain::TranslateXarchArgs(
     llvm::opt::DerivedArgList *DAL,
     SmallVectorImpl<llvm::opt::Arg *> *AllocatedArgs) const {
   const OptTable &Opts = getDriver().getOpts();
   unsigned ValuePos = 1;
   if (A->getOption().matches(options::OPT_Xarch_device) ||
       A->getOption().matches(options::OPT_Xarch_host))
     ValuePos = 0;
 
-  unsigned Index = Args.getBaseArgs().MakeIndex(A->getValue(ValuePos));
-  unsigned Prev = Index;
-  std::unique_ptr<llvm::opt::Arg> XarchArg(Opts.ParseOneArg(Args, Index));
-
-  // If the argument parsing failed or more than one argument was
-  // consumed, the -Xarch_ argument's parameter tried to consume
-  // extra arguments. Emit an error and ignore.
+  std::string XarchArgValues(A->getValue(ValuePos));
+  std::stringstream SS(XarchArgValues);
+  std::string S;
+  unsigned FirstIndex = ~0U;
+  unsigned LastIndex = 0;
+  while (SS >> S) {
+    unsigned Index = Args.getBaseArgs().MakeIndex(S);
+    if (FirstIndex == ~0U)
+      FirstIndex = Index;
+    LastIndex = Index;
+  }
+  unsigned Index = FirstIndex;
+  std::unique_ptr<llvm::opt::Arg> XarchArg(
+      Opts.ParseOneArg(Args.getBaseArgs(), Index, LastIndex + 1));
+
+  // If the argument parsing failed or incorrect number of arguments were
+  // provided, emit an error and ignore.
   //
   // We also want to disallow any options which would alter the
   // driver behavior; that isn't going to work in our model. We
   // use options::NoXarchOption to control this.
-  if (!XarchArg || Index > Prev + 1) {
+  if (!XarchArg || Index != LastIndex + 1) {
     getDriver().Diag(diag::err_drv_invalid_Xarch_argument_with_args)
         << A->getAsString(Args);
     return;
   } else if (XarchArg->getOption().hasFlag(options::NoXarchOption)) {
     auto &Diags = getDriver().getDiags();
     unsigned DiagID =
         Diags.getCustomDiagID(DiagnosticsEngine::Error,
                               "invalid Xarch argument: '%0', not all driver "

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

TransferableCommand(CompileCommand C)
     auto &OptTable = clang::driver::getDriverOptTable();
     if (!OldArgs.empty())
       Cmd.CommandLine.emplace_back(OldArgs.front());
     for (unsigned Pos = 1; Pos < OldArgs.size();) {
       using namespace driver::options;
 
       const unsigned OldPos = Pos;
       std::unique_ptr<llvm::opt::Arg> Arg(OptTable.ParseOneArg(
-          ArgList, Pos,
+          ArgList, Pos, ArgList.getNumInputArgStrings(),
           /* Include */ ClangCLMode ? CoreOption | CLOption | CLDXCOption : 0,
           /* Exclude */ ClangCLMode ? 0 : CLOption | CLDXCOption));
 
       if (!Arg)
         continue;
 
       const llvm::opt::Option &Opt = Arg->getOption();
 

clang/test/Driver/hip-options.hip

 
 // RUN: %clang -### -Xarch_device -g -nogpulib --cuda-gpu-arch=gfx900 \
 // RUN:   -Xarch_device -fcf-protection=branch \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=DEV %s
 // DEV: "-cc1"{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
 // DEV: "-cc1"{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
 // DEV-NOT: clang{{.*}} {{.*}} "-debug-info-kind={{.*}}"
 
+// RUN: %clang -### -nogpulib --cuda-gpu-arch=gfx900 \
+// RUN:   -Xarch_device "-mllvm --inline-threshold=100" \
+// RUN:   -Xarch_device "-mllvm --inline-hint-threshold=200" \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=LLVM %s
+// LLVM: "-cc1"{{.*}} "-fcuda-is-device" {{.*}}"-mllvm" "--inline-threshold=100" {{.*}}"-mllvm" "--inline-hint-threshold=200"
+// LLVM: "-cc1"{{.*}} "-fcuda-is-device" {{.*}} "-mllvm" "--inline-threshold=100" {{.*}}"-mllvm" "--inline-hint-threshold=200"
+// LLVM-NOT: clang{{.*}}"-mllvm" "--inline-threshold=100"
+// LLVM-NOT: clang{{.*}}"-mllvm" "--inline-hint-threshold=200"
+
 // RUN: %clang -### -Xarch_host -g -nogpulib --cuda-gpu-arch=gfx900 \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=HOST %s
 // HOST-NOT: "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
 // HOST-NOT: "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
 // HOST: clang{{.*}} "-debug-info-kind={{.*}}"
 
 // RUN: %clang -### -nogpuinc -nogpulib -munsafe-fp-atomics \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=UNSAFE-FP-ATOMICS %s

llvm/include/llvm/Option/OptTable.h

 
 private:
   const Info &getInfo(OptSpecifier Opt) const {
     unsigned id = Opt.getID();
     assert(id > 0 && id - 1 < getNumOptions() && "Invalid Option ID.");
     return OptionInfos[id - 1];
   }
 
-  std::unique_ptr<Arg> parseOneArgGrouped(InputArgList &Args,
-                                          unsigned &Index) const;
+  std::unique_ptr<Arg> parseOneArgGrouped(InputArgList &Args, unsigned &Index,
+                                          unsigned IndexEnd) const;
 
 protected:
   /// Initialize OptTable using Tablegen'ed OptionInfos. Child class must
   /// manually call \c buildPrefixChars once they are fully constructed.
   OptTable(ArrayRef<Info> OptionInfos, bool IgnoreCase = false);
 
   /// Build (or rebuild) the PrefixChars member.
   void buildPrefixChars();
public:
   }
 
   /// Parse a single argument; returning the new argument and
   /// updating Index.
   ///
   /// \param [in,out] Index - The current parsing position in the argument
   /// string list; on return this will be the index of the next argument
   /// string to parse.
-  /// \param [in] FlagsToInclude - Only parse options with any of these flags.
-  /// Zero is the default which includes all flags.
-  /// \param [in] FlagsToExclude - Don't parse options with this flag.  Zero
-  /// is the default and means exclude nothing.
+  /// \param [in] IndexEnd - The non-inclusive end parsing position in the
+  /// argument string list. \param [in] FlagsToInclude - Only parse options with
+  /// any of these flags. Zero is the default which includes all flags. \param
+  /// [in] FlagsToExclude - Don't parse options with this flag.  Zero is the
+  /// default and means exclude nothing.
   ///
   /// \return The parsed argument, or 0 if the argument is missing values
   /// (in which case Index still points at the conceptual next argument string
   /// to parse).
   std::unique_ptr<Arg> ParseOneArg(const ArgList &Args, unsigned &Index,
+                                   unsigned IndexEnd,
                                    unsigned FlagsToInclude = 0,
                                    unsigned FlagsToExclude = 0) const;
 
   /// Parse an list of arguments into an InputArgList.
   ///
   /// The resulting InputArgList will reference the strings in [\p ArgBegin,
   /// \p ArgEnd), and their lifetime should extend past that of the returned
   /// InputArgList.

llvm/include/llvm/Option/Option.h

public:
   /// If the option accepts the current argument, accept() sets
   /// Index to the position where argument parsing should resume
   /// (even if the argument is missing values).
   ///
   /// \p CurArg The argument to be matched. It may be shorter than the
   /// underlying storage to represent a Joined argument.
   /// \p GroupedShortOption If true, we are handling the fallback case of
   /// parsing a prefix of the current argument as a short option.
+  /// \p Index The start parsing position.
+  /// \p IndexEnd The end parsing position, non-inclusive.
   std::unique_ptr<Arg> accept(const ArgList &Args, StringRef CurArg,
-                              bool GroupedShortOption, unsigned &Index) const;
+                              bool GroupedShortOption, unsigned &Index,
+                              unsigned IndexEnd) const;
 
 private:
   std::unique_ptr<Arg> acceptInternal(const ArgList &Args, StringRef CurArg,
-                                      unsigned &Index) const;
+                                      unsigned &Index, unsigned IndexEnd) const;
 
 public:
   void print(raw_ostream &O) const;
   void dump() const;
 };
 
 } // end namespace opt
 
 } // end namespace llvm
 
 #endif // LLVM_OPTION_OPTION_H

llvm/lib/Option/OptTable.cpp

unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
   return BestDistance;
 }
 
 // Parse a single argument, return the new argument, and update Index. If
 // GroupedShortOptions is true, -a matches "-abc" and the argument in Args will
 // be updated to "-bc". This overload does not support
 // FlagsToInclude/FlagsToExclude or case insensitive options.
 std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,
-                                                  unsigned &Index) const {
+                                                  unsigned &Index,
+                                                  unsigned IndexEnd) const {
   // Anything that doesn't start with PrefixesUnion is an input, as is '-'
   // itself.
   const char *CStr = Args.getArgString(Index);
   StringRef Str(CStr);
   if (isInput(getPrefixesUnion(), Str))
     return std::make_unique<Arg>(getOption(InputOptionID), Str, Index++, CStr);
 
   const Info *End = OptionInfos.data() + OptionInfos.size();
   StringRef Name = Str.ltrim(PrefixChars);
   const Info *Start =
       std::lower_bound(OptionInfos.data() + FirstSearchableIndex, End, Name);
   const Info *Fallback = nullptr;
   unsigned Prev = Index;
 
   // Search for the option which matches Str.
   for (; Start != End; ++Start) {
     unsigned ArgSize = matchOption(Start, Str, IgnoreCase);
     if (!ArgSize)
       continue;
 
     Option Opt(Start, this);
     if (std::unique_ptr<Arg> A =
             Opt.accept(Args, StringRef(Args.getArgString(Index), ArgSize),
-                       /*GroupedShortOption=*/false, Index))
+                       /*GroupedShortOption=*/false, Index, IndexEnd))
       return A;
 
     // If Opt is a Flag of length 2 (e.g. "-a"), we know it is a prefix of
     // the current argument (e.g. "-abc"). Match it as a fallback if no longer
     // option (e.g. "-ab") exists.
     if (ArgSize == 2 && Opt.getKind() == Option::FlagClass)
       Fallback = Start;
 
     // Otherwise, see if the argument is missing.
     if (Prev != Index)
       return nullptr;
   }
   if (Fallback) {
     Option Opt(Fallback, this);
     // Check that the last option isn't a flag wrongly given an argument.
     if (Str[2] == '=')
       return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++,
                                    CStr);
 
-    if (std::unique_ptr<Arg> A = Opt.accept(
-            Args, Str.substr(0, 2), /*GroupedShortOption=*/true, Index)) {
+    if (std::unique_ptr<Arg> A =
+            Opt.accept(Args, Str.substr(0, 2), /*GroupedShortOption=*/true,
+                       Index, IndexEnd)) {
       Args.replaceArgString(Index, Twine('-') + Str.substr(2));
       return A;
     }
   }
 
   // In the case of an incorrect short option extract the character and move to
   // the next one.
   if (Str[1] != '-') {
     CStr = Args.MakeArgString(Str.substr(0, 2));
     Args.replaceArgString(Index, Twine('-') + Str.substr(2));
     return std::make_unique<Arg>(getOption(UnknownOptionID), CStr, Index, CStr);
   }
 
   return std::make_unique<Arg>(getOption(UnknownOptionID), Str, Index++, CStr);
 }
 
 std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
+                                           unsigned IndexEnd,
                                            unsigned FlagsToInclude,
                                            unsigned FlagsToExclude) const {
   unsigned Prev = Index;
   StringRef Str = Args.getArgString(Index);
 
   // Anything that doesn't start with PrefixesUnion is an input, as is '-'
   // itself.
   if (isInput(getPrefixesUnion(), Str))
for (; Start != End; ++Start) {
     if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
       continue;
     if (Opt.hasFlag(FlagsToExclude))
       continue;
 
     // See if this option matches.
     if (std::unique_ptr<Arg> A =
             Opt.accept(Args, StringRef(Args.getArgString(Index), ArgSize),
-                       /*GroupedShortOption=*/false, Index))
+                       /*GroupedShortOption=*/false, Index, IndexEnd))
       return A;
 
     // Otherwise, see if this argument was missing values.
     if (Prev != Index)
       return nullptr;
   }
 
   // If we failed to find an option and this arg started with /, then it's
while (Index < End) {
     // Ignore empty arguments (other things may still take them as arguments).
     StringRef Str = Args.getArgString(Index);
     if (Str == "") {
       ++Index;
       continue;
     }
 
     unsigned Prev = Index;
-    std::unique_ptr<Arg> A = GroupedShortOptions
-                 ? parseOneArgGrouped(Args, Index)
-                 : ParseOneArg(Args, Index, FlagsToInclude, FlagsToExclude);
+    std::unique_ptr<Arg> A =
+        GroupedShortOptions
+            ? parseOneArgGrouped(Args, Index, End)
+            : ParseOneArg(Args, Index, End, FlagsToInclude, FlagsToExclude);
     assert((Index > Prev || GroupedShortOptions) &&
            "Parser failed to consume argument.");
 
     // Check for missing argument error.
     if (!A) {
       assert(Index >= End && "Unexpected parser error.");
       assert(Index - Prev - 1 && "No missing arguments!");
       MissingArgIndex = Prev;

llvm/lib/Option/Option.cpp

bool Option::matches(OptSpecifier Opt) const {
 
   const Option Group = getGroup();
   if (Group.isValid())
     return Group.matches(Opt);
   return false;
 }
 
 std::unique_ptr<Arg> Option::acceptInternal(const ArgList &Args,
-                                            StringRef Spelling,
-                                            unsigned &Index) const {
+                                            StringRef Spelling, unsigned &Index,
+                                            unsigned IndexEnd) const {
   size_t ArgSize = Spelling.size();
   switch (getKind()) {
   case FlagClass: {
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
     return std::make_unique<Arg>(*this, Spelling, Index++);
   }
   case JoinedClass: {
std::unique_ptr<Arg> Option::acceptInternal(const ArgList &Args,
   }
   case SeparateClass:
     // Matches iff this is an exact match.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
 
     Index += 2;
-    if (Index > Args.getNumInputArgStrings() ||
-        Args.getArgString(Index - 1) == nullptr)
+    if (Index > IndexEnd || Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
     return std::make_unique<Arg>(*this, Spelling, Index - 2,
                                  Args.getArgString(Index - 1));
   case MultiArgClass: {
     // Matches iff this is an exact match.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
 
     Index += 1 + getNumArgs();
-    if (Index > Args.getNumInputArgStrings())
+    if (Index > IndexEnd)
       return nullptr;
 
     auto A = std::make_unique<Arg>(*this, Spelling, Index - 1 - getNumArgs(),
                                    Args.getArgString(Index - getNumArgs()));
     for (unsigned i = 1; i != getNumArgs(); ++i)
       A->getValues().push_back(Args.getArgString(Index - getNumArgs() + i));
     return A;
   }
   case JoinedOrSeparateClass: {
     // If this is not an exact match, it is a joined arg.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index))) {
       const char *Value = Args.getArgString(Index) + ArgSize;
       return std::make_unique<Arg>(*this, Spelling, Index++, Value);
     }
 
     // Otherwise it must be separate.
     Index += 2;
-    if (Index > Args.getNumInputArgStrings() ||
-        Args.getArgString(Index - 1) == nullptr)
+    if (Index > IndexEnd || Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
     return std::make_unique<Arg>(*this, Spelling, Index - 2,
                                  Args.getArgString(Index - 1));
   }
   case JoinedAndSeparateClass:
     // Always matches.
     Index += 2;
-    if (Index > Args.getNumInputArgStrings() ||
-        Args.getArgString(Index - 1) == nullptr)
+    if (Index > IndexEnd || Args.getArgString(Index - 1) == nullptr)
       return nullptr;
 
     return std::make_unique<Arg>(*this, Spelling, Index - 2,
                                  Args.getArgString(Index - 2) + ArgSize,
                                  Args.getArgString(Index - 1));
   case RemainingArgsClass: {
     // Matches iff this is an exact match.
     // FIXME: Avoid strlen.
     if (ArgSize != strlen(Args.getArgString(Index)))
       return nullptr;
     auto A = std::make_unique<Arg>(*this, Spelling, Index++);
-    while (Index < Args.getNumInputArgStrings() &&
-           Args.getArgString(Index) != nullptr)
+    while (Index < IndexEnd && Args.getArgString(Index) != nullptr)
       A->getValues().push_back(Args.getArgString(Index++));
     return A;
   }
   case RemainingArgsJoinedClass: {
     auto A = std::make_unique<Arg>(*this, Spelling, Index);
     if (ArgSize != strlen(Args.getArgString(Index))) {
       // An inexact match means there is a joined arg.
       A->getValues().push_back(Args.getArgString(Index) + ArgSize);
     }
     Index++;
-    while (Index < Args.getNumInputArgStrings() &&
-           Args.getArgString(Index) != nullptr)
+    while (Index < IndexEnd && Args.getArgString(Index) != nullptr)
       A->getValues().push_back(Args.getArgString(Index++));
     return A;
   }
 
   default:
     llvm_unreachable("Invalid option kind!");
   }
 }
 
 std::unique_ptr<Arg> Option::accept(const ArgList &Args, StringRef CurArg,
-                                    bool GroupedShortOption,
-                                    unsigned &Index) const {
+                                    bool GroupedShortOption, unsigned &Index,
+                                    unsigned IndexEnd) const {
   auto A(GroupedShortOption && getKind() == FlagClass
-                             ? std::make_unique<Arg>(*this, CurArg, Index)
-                             : acceptInternal(Args, CurArg, Index));
+             : acceptInternal(Args, CurArg, Index, IndexEnd));
   if (!A)
     return nullptr;
 
   const Option &UnaliasedOption = getUnaliasedOption();
   if (getID() == UnaliasedOption.getID())
     return A;
 
   // "A" is an alias for a different flag. For most clients it's more convenient