yaxunl created this revision.
yaxunl added reviewers: tra, MaskRay.
Herald added a subscriber: hiraditya.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, sstefan1.
Herald added a project: LLVM.
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.
https://reviews.llvm.org/D143305
Files:
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
clang/test/Driver/hip-options.hip
llvm/include/llvm/Option/OptTable.h
llvm/include/llvm/Option/Option.h
llvm/lib/Option/OptTable.cpp
llvm/lib/Option/Option.cpp
Index: llvm/lib/Option/Option.cpp
===================================================================
--- llvm/lib/Option/Option.cpp
+++ llvm/lib/Option/Option.cpp
@@ -106,8 +106,8 @@
}
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: {
@@ -154,8 +154,7 @@
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,
@@ -167,7 +166,7 @@
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(),
@@ -186,8 +185,7 @@
// 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,
@@ -196,8 +194,7 @@
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,
@@ -209,8 +206,7 @@
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;
}
@@ -221,8 +217,7 @@
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;
}
@@ -233,11 +228,11 @@
}
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));
+ ? std::make_unique<Arg>(*this, CurArg, Index)
+ : acceptInternal(Args, CurArg, Index, IndexEnd));
if (!A)
return nullptr;
Index: llvm/lib/Option/OptTable.cpp
===================================================================
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -318,7 +318,8 @@
// 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);
@@ -342,7 +343,7 @@
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
@@ -362,8 +363,9 @@
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;
}
@@ -381,6 +383,7 @@
}
std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
+ unsigned IndexEnd,
unsigned FlagsToInclude,
unsigned FlagsToExclude) const {
unsigned Prev = Index;
@@ -426,7 +429,7 @@
// 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.
@@ -469,9 +472,10 @@
}
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.");
Index: llvm/include/llvm/Option/Option.h
===================================================================
--- llvm/include/llvm/Option/Option.h
+++ llvm/include/llvm/Option/Option.h
@@ -218,12 +218,15 @@
/// 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;
Index: llvm/include/llvm/Option/OptTable.h
===================================================================
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -86,8 +86,8 @@
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
@@ -197,15 +197,17 @@
/// \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;
Index: clang/test/Driver/hip-options.hip
===================================================================
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -33,6 +33,15 @@
// 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={{.*}}"
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -163,7 +163,7 @@
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));
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -44,6 +44,7 @@
#include <cassert>
#include <cstddef>
#include <cstring>
+#include <sstream>
#include <string>
using namespace clang;
@@ -1256,7 +1257,8 @@
// 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);
@@ -1294,18 +1296,28 @@
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));
+ 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 more than one argument was
- // consumed, the -Xarch_ argument's parameter tried to consume
- // extra arguments. Emit an error and ignore.
+ // 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;
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4301,7 +4301,8 @@
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();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
