[llvm] b2eda85 - [OptTable] Make explicitly included options override excluded ones
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 19 15:29:24 PDT 2023
Author: Justin Bogner
Date: 2023-07-19T15:28:34-07:00
New Revision: b2eda85f047f27788ccd7b9af9bd59c5d44b2051
URL: https://github.com/llvm/llvm-project/commit/b2eda85f047f27788ccd7b9af9bd59c5d44b2051
DIFF: https://github.com/llvm/llvm-project/commit/b2eda85f047f27788ccd7b9af9bd59c5d44b2051.diff
LOG: [OptTable] Make explicitly included options override excluded ones
When we have both explicitly included and excluded option sets, we
were excluding anything from the latter set regardless of what was in
the former. This doesn't compose well and led to an overly complicated
design around DXC options where a third flag was introduced to handle
options that overlapped between DXC and CL.
With this change we check the included options before excluding
anything from the exclude list, which allows for options that are in
multiple categories to be handled in a sensible way. This allows us to
remove CLDXCOption but should otherwise be NFC.
Differential Revision: https://reviews.llvm.org/D155729
Added:
Modified:
clang-tools-extra/clangd/CompileCommands.cpp
clang/include/clang/Driver/Options.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
llvm/lib/Option/OptTable.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index 957a6f873e1022..80d90ff27648e3 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -238,13 +238,10 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
ArgList = OptTable.ParseArgs(
llvm::ArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount,
/*FlagsToInclude=*/
- IsCLMode ? (driver::options::CLOption | driver::options::CoreOption |
- driver::options::CLDXCOption)
+ IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)
: /*everything*/ 0,
/*FlagsToExclude=*/driver::options::NoDriverOption |
- (IsCLMode
- ? 0
- : (driver::options::CLOption | driver::options::CLDXCOption)));
+ (IsCLMode ? 0 : driver::options::CLOption));
llvm::SmallVector<unsigned, 1> IndicesToDrop;
// Having multiple architecture options (e.g. when building fat binaries)
@@ -449,8 +446,6 @@ unsigned char getModes(const llvm::opt::Option &Opt) {
if (!Opt.hasFlag(driver::options::NoDriverOption)) {
if (Opt.hasFlag(driver::options::CLOption)) {
Result |= DM_CL;
- } else if (Opt.hasFlag(driver::options::CLDXCOption)) {
- Result |= DM_CL;
} else {
Result |= DM_GCC;
if (Opt.hasFlag(driver::options::CoreOption)) {
diff --git a/clang/include/clang/Driver/Options.h b/clang/include/clang/Driver/Options.h
index 54c6f5faa37c22..58f5df132feca8 100644
--- a/clang/include/clang/Driver/Options.h
+++ b/clang/include/clang/Driver/Options.h
@@ -36,9 +36,8 @@ enum ClangFlags {
FC1Option = (1 << 15),
FlangOnlyOption = (1 << 16),
DXCOption = (1 << 17),
- CLDXCOption = (1 << 18),
- Ignored = (1 << 19),
- TargetSpecific = (1 << 20),
+ Ignored = (1 << 18),
+ TargetSpecific = (1 << 19),
};
enum ID {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 8ffb9388d330f0..90fa3c6dabdde4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -53,10 +53,6 @@ def CC1AsOption : OptionFlag;
// are made available when the driver is running in DXC compatibility mode.
def DXCOption : OptionFlag;
-// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with this flag
-// are made available when the driver is running in CL/DXC compatibility mode.
-def CLDXCOption : OptionFlag;
-
// NoDriverOption - This option should not be accepted by the driver.
def NoDriverOption : OptionFlag;
@@ -6856,7 +6852,7 @@ def defsym : Separate<["-"], "defsym">,
// clang-cl Options
//===----------------------------------------------------------------------===//
-def cl_Group : OptionGroup<"<clang-cl options>">, Flags<[CLDXCOption]>,
+def cl_Group : OptionGroup<"<clang-cl options>">,
HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
def cl_compile_Group : OptionGroup<"<clang-cl compile-only options>">,
@@ -6865,42 +6861,45 @@ def cl_compile_Group : OptionGroup<"<clang-cl compile-only options>">,
def cl_ignored_Group : OptionGroup<"<clang-cl ignored options>">,
Group<cl_Group>;
-class CLFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
- Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
-
-class CLDXCFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
- Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
-
-class CLCompileFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
- Group<cl_compile_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLFlagOpts<list<OptionFlag> flags>
+ : Flags<!listconcat(flags, [NoXarchOption])>;
-class CLIgnoredFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
- Group<cl_ignored_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLFlag<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_FLAG>,
+ Group<cl_Group>, CLFlagOpts<flags>;
-class CLJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
- Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLCompileFlag<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_FLAG>,
+ Group<cl_compile_Group>, CLFlagOpts<flags>;
-class CLDXCJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
- Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
+class CLIgnoredFlag<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_FLAG>,
+ Group<cl_ignored_Group>, CLFlagOpts<flags>;
-class CLCompileJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
- Group<cl_compile_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLJoined<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_JOINED>,
+ Group<cl_Group>, CLFlagOpts<flags>;
-class CLIgnoredJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
- Group<cl_ignored_Group>, Flags<[CLOption, NoXarchOption, HelpHidden]>;
+class CLCompileJoined<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_JOINED>,
+ Group<cl_compile_Group>, CLFlagOpts<flags>;
-class CLJoinedOrSeparate<string name> : Option<["/", "-"], name,
- KIND_JOINED_OR_SEPARATE>, Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLIgnoredJoined<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_JOINED>,
+ Group<cl_ignored_Group>, CLFlagOpts<!listconcat(flags, [HelpHidden])>;
-class CLDXCJoinedOrSeparate<string name> : Option<["/", "-"], name,
- KIND_JOINED_OR_SEPARATE>, Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
+class CLJoinedOrSeparate<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_JOINED_OR_SEPARATE>,
+ Group<cl_Group>, CLFlagOpts<flags>;
-class CLCompileJoinedOrSeparate<string name> : Option<["/", "-"], name,
- KIND_JOINED_OR_SEPARATE>, Group<cl_compile_Group>,
- Flags<[CLOption, NoXarchOption]>;
+class CLCompileJoinedOrSeparate<string name,
+ list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_JOINED_OR_SEPARATE>,
+ Group<cl_compile_Group>, CLFlagOpts<flags>;
-class CLRemainingArgsJoined<string name> : Option<["/", "-"], name,
- KIND_REMAINING_ARGS_JOINED>, Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLRemainingArgsJoined<string name, list<OptionFlag> flags = [CLOption]>
+ : Option<["/", "-"], name, KIND_REMAINING_ARGS_JOINED>,
+ Group<cl_Group>, CLFlagOpts<flags>;
// Aliases:
// (We don't put any of these in cl_compile_Group as the options they alias are
@@ -6971,7 +6970,7 @@ def _SLASH_help : CLFlag<"help">, Alias<help>,
def _SLASH_HELP : CLFlag<"HELP">, Alias<help>;
def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias<fms_hotpatch>,
HelpText<"Create hotpatchable image">;
-def _SLASH_I : CLDXCJoinedOrSeparate<"I">,
+def _SLASH_I : CLJoinedOrSeparate<"I", [CLOption, DXCOption]>,
HelpText<"Add directory to include search path">, MetaVarName<"<dir>">,
Alias<I>;
def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
@@ -6979,7 +6978,7 @@ def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
// The _SLASH_O option handles all the /O flags, but we also provide separate
// aliased options to provide separate help messages.
-def _SLASH_O : CLDXCJoined<"O">,
+def _SLASH_O : CLJoined<"O", [CLOption, DXCOption]>,
HelpText<"Set multiple /O flags at once; e.g. '/O2y-' for '/O2 /Oy-'">,
MetaVarName<"<flags>">;
def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>,
@@ -6992,7 +6991,7 @@ def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>,
HelpText<"Only inline functions explicitly or implicitly marked inline">;
def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>,
HelpText<"Inline functions as deemed beneficial by the compiler">;
-def : CLDXCFlag<"Od">, Alias<_SLASH_O>, AliasArgs<["d"]>,
+def : CLFlag<"Od", [CLOption, DXCOption]>, Alias<_SLASH_O>, AliasArgs<["d"]>,
HelpText<"Disable optimization">;
def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>,
HelpText<"No effect">;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index fc52f47df00b5d..3e511fb974ca2d 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6496,7 +6496,6 @@ Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
if (IsClCompatMode) {
// Include CL and Core options.
IncludedFlagsBitmask |= options::CLOption;
- IncludedFlagsBitmask |= options::CLDXCOption;
IncludedFlagsBitmask |= options::CoreOption;
} else {
ExcludedFlagsBitmask |= options::CLOption;
@@ -6504,13 +6503,10 @@ Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
if (IsDXCMode()) {
// Include DXC and Core options.
IncludedFlagsBitmask |= options::DXCOption;
- IncludedFlagsBitmask |= options::CLDXCOption;
IncludedFlagsBitmask |= options::CoreOption;
} else {
ExcludedFlagsBitmask |= options::DXCOption;
}
- if (!IsClCompatMode && !IsDXCMode())
- ExcludedFlagsBitmask |= options::CLDXCOption;
return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
}
diff --git a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
index ff9cb4b62073d7..1e9b9f93751942 100644
--- a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -164,8 +164,8 @@ struct TransferableCommand {
const unsigned OldPos = Pos;
std::unique_ptr<llvm::opt::Arg> Arg(OptTable.ParseOneArg(
ArgList, Pos,
- /* Include */ ClangCLMode ? CoreOption | CLOption | CLDXCOption : 0,
- /* Exclude */ ClangCLMode ? 0 : CLOption | CLDXCOption));
+ /* Include */ ClangCLMode ? CoreOption | CLOption : 0,
+ /* Exclude */ ClangCLMode ? 0 : CLOption));
if (!Arg)
continue;
diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 3f53ac119c69e0..6268a5c41b4ef6 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -421,7 +421,8 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
continue;
if (Opt.hasFlag(FlagsToExclude))
- continue;
+ if (!FlagsToInclude || !Opt.hasFlag(FlagsToInclude))
+ continue;
// See if this option matches.
if (std::unique_ptr<Arg> A =
@@ -650,7 +651,8 @@ void OptTable::printHelp(raw_ostream &OS, const char *Usage, const char *Title,
if (FlagsToInclude && !(Flags & FlagsToInclude))
continue;
if (Flags & FlagsToExclude)
- continue;
+ if (!FlagsToInclude || !(Flags & FlagsToInclude))
+ continue;
// If an alias doesn't have a help text, show a help text for the aliased
// option instead.
More information about the llvm-commits
mailing list