[clang][DepScan] Make OptimizeArgs a bit mask enum and enable by default (#71588)

Bigcheese · web-flow · commit fb07d9cc096f · 2023-11-07T16:06:59.000-08:00
Make it easier to control which optimizations are enabled by making
OptimizeArgs a bit masked enum. There's currently only one such
optimization, but more will be added in followup commits.
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
+#include "llvm/ADT/BitmaskEnum.h"
 
 namespace clang {
 namespace tooling {
@@ -44,19 +45,31 @@ enum class ScanningOutputFormat {
   P1689,
 };
 
+enum class ScanningOptimizations {
+  None = 0,
+
+  /// Remove unused header search paths including header maps.
+  HeaderSearch = 1,
+
+  LLVM_MARK_AS_BITMASK_ENUM(HeaderSearch),
+  All = HeaderSearch,
+  Default = All
+};
+
 /// The dependency scanning service contains shared configuration and state that
 /// is used by the individual dependency scanning workers.
 class DependencyScanningService {
 public:
-  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
-                            bool OptimizeArgs = false,
-                            bool EagerLoadModules = false);
+  DependencyScanningService(
+      ScanningMode Mode, ScanningOutputFormat Format,
+      ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
+      bool EagerLoadModules = false);
 
   ScanningMode getMode() const { return Mode; }
 
   ScanningOutputFormat getFormat() const { return Format; }
 
-  bool canOptimizeArgs() const { return OptimizeArgs; }
+  ScanningOptimizations getOptimizeArgs() const { return OptimizeArgs; }
 
   bool shouldEagerLoadModules() const { return EagerLoadModules; }
 
@@ -68,7 +81,7 @@ class DependencyScanningService {
   const ScanningMode Mode;
   const ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.
-  const bool OptimizeArgs;
+  const ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   const bool EagerLoadModules;
   /// The global file system cache.
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -116,7 +116,7 @@ class DependencyScanningWorker {
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.
-  bool OptimizeArgs;
+  ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   bool EagerLoadModules;
 };
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -16,6 +16,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringSet.h"
@@ -211,8 +212,9 @@ class ModuleDepCollector final : public DependencyCollector {
   ModuleDepCollector(std::unique_ptr<DependencyOutputOptions> Opts,
                      CompilerInstance &ScanInstance, DependencyConsumer &C,
                      DependencyActionController &Controller,
-                     CompilerInvocation OriginalCI, bool OptimizeArgs,
-                     bool EagerLoadModules, bool IsStdModuleP1689Format);
+                     CompilerInvocation OriginalCI,
+                     ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
+                     bool IsStdModuleP1689Format);
 
   void attachToPreprocessor(Preprocessor &PP) override;
   void attachToASTReader(ASTReader &R) override;
@@ -254,7 +256,7 @@ class ModuleDepCollector final : public DependencyCollector {
   /// for each individual module.
   CowCompilerInvocation CommonInvocation;
   /// Whether to optimize the modules' command-line arguments.
-  bool OptimizeArgs;
+  ScanningOptimizations OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.
   bool EagerLoadModules;
   /// If we're generating dependency output in P1689 format
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -14,8 +14,8 @@ using namespace tooling;
 using namespace dependencies;
 
 DependencyScanningService::DependencyScanningService(
-    ScanningMode Mode, ScanningOutputFormat Format, bool OptimizeArgs,
-    bool EagerLoadModules)
+    ScanningMode Mode, ScanningOutputFormat Format,
+    ScanningOptimizations OptimizeArgs, bool EagerLoadModules)
     : Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
       EagerLoadModules(EagerLoadModules) {
   // Initialize targets for object file support.
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -137,8 +137,9 @@ class DependencyScanningAction : public tooling::ToolAction {
       StringRef WorkingDirectory, DependencyConsumer &Consumer,
       DependencyActionController &Controller,
       llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
-      ScanningOutputFormat Format, bool OptimizeArgs, bool EagerLoadModules,
-      bool DisableFree, std::optional<StringRef> ModuleName = std::nullopt)
+      ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs,
+      bool EagerLoadModules, bool DisableFree,
+      std::optional<StringRef> ModuleName = std::nullopt)
       : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
         Controller(Controller), DepFS(std::move(DepFS)), Format(Format),
         OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules),
@@ -297,7 +298,7 @@ class DependencyScanningAction : public tooling::ToolAction {
   DependencyActionController &Controller;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
-  bool OptimizeArgs;
+  ScanningOptimizations OptimizeArgs;
   bool EagerLoadModules;
   bool DisableFree;
   std::optional<StringRef> ModuleName;
@@ -312,7 +313,7 @@ class DependencyScanningAction : public tooling::ToolAction {
 DependencyScanningWorker::DependencyScanningWorker(
     DependencyScanningService &Service,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
-    : Format(Service.getFormat()), OptimizeArgs(Service.canOptimizeArgs()),
+    : Format(Service.getFormat()), OptimizeArgs(Service.getOptimizeArgs()),
       EagerLoadModules(Service.shouldEagerLoadModules()) {
   PCHContainerOps = std::make_shared<PCHContainerOperations>();
   // We need to read object files from PCH built outside the scanner.
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -529,7 +529,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   CowCompilerInvocation CI =
       MDC.getInvocationAdjustedForModuleBuildWithoutOutputs(
           MD, [&](CowCompilerInvocation &BuildInvocation) {
-            if (MDC.OptimizeArgs)
+            if (any(MDC.OptimizeArgs & ScanningOptimizations::HeaderSearch))
               optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
                                        *MDC.ScanInstance.getASTReader(), *MF);
           });
@@ -628,7 +628,8 @@ ModuleDepCollector::ModuleDepCollector(
     std::unique_ptr<DependencyOutputOptions> Opts,
     CompilerInstance &ScanInstance, DependencyConsumer &C,
     DependencyActionController &Controller, CompilerInvocation OriginalCI,
-    bool OptimizeArgs, bool EagerLoadModules, bool IsStdModuleP1689Format)
+    ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
+    bool IsStdModuleP1689Format)
     : ScanInstance(ScanInstance), Consumer(C), Controller(Controller),
       Opts(std::move(Opts)),
       CommonInvocation(
diff --git a/clang/test/ClangScanDeps/header-search-pruning-transitive.c b/clang/test/ClangScanDeps/header-search-pruning-transitive.c
@@ -54,8 +54,8 @@ module X { header "X.h" }
 // RUN: sed -e "s|DIR|%/t|g" %t/cdb_with_a.json.template    > %t/cdb_with_a.json
 // RUN: sed -e "s|DIR|%/t|g" %t/cdb_without_a.json.template > %t/cdb_without_a.json
 
-// RUN: clang-scan-deps -compilation-database %t/cdb_with_a.json    -format experimental-full -optimize-args >  %t/results.json
-// RUN: clang-scan-deps -compilation-database %t/cdb_without_a.json -format experimental-full -optimize-args >> %t/results.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_with_a.json    -format experimental-full -optimize-args=header-search >  %t/results.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_without_a.json -format experimental-full -optimize-args=header-search >> %t/results.json
 // RUN: cat %t/results.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
 
 // CHECK:      {
diff --git a/clang/test/ClangScanDeps/header-search-pruning.cpp b/clang/test/ClangScanDeps/header-search-pruning.cpp
@@ -5,13 +5,13 @@
 // RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_B|g"             %S/Inputs/header-search-pruning/cdb.json > %t/cdb_b.json
 // RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_A -DINCLUDE_B|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_ab.json
 //
-// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -optimize-args >> %t/result_a.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -optimize-args=header-search >> %t/result_a.json
 // RUN: cat %t/result_a.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_A %s
 //
-// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -optimize-args >> %t/result_b.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -optimize-args=header-search >> %t/result_b.json
 // RUN: cat %t/result_b.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_B %s
 //
-// RUN: clang-scan-deps -compilation-database %t/cdb_ab.json -format experimental-full -optimize-args >> %t/result_ab.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_ab.json -format experimental-full -optimize-args=header-search >> %t/result_ab.json
 // RUN: cat %t/result_ab.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_AB %s
 
 #include "mod.h"
diff --git a/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c b/clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
@@ -12,7 +12,7 @@
 
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 \
 // RUN:   -format experimental-full  -mode=preprocess-dependency-directives \
-// RUN:   -optimize-args -module-files-dir %t/build > %t/deps.json
+// RUN:   -optimize-args=all -module-files-dir %t/build > %t/deps.json
 
 // RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
 
diff --git a/clang/test/ClangScanDeps/modules-symlink-dir.c b/clang/test/ClangScanDeps/modules-symlink-dir.c
@@ -14,7 +14,7 @@
 
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 \
 // RUN:   -format experimental-full  -mode=preprocess-dependency-directives \
-// RUN:   -optimize-args -module-files-dir %t/build > %t/deps.json
+// RUN:   -optimize-args=all -module-files-dir %t/build > %t/deps.json
 
 // RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
 
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -75,8 +75,8 @@ enum ResourceDirRecipeKind {
 
 static ScanningMode ScanMode = ScanningMode::DependencyDirectivesScan;
 static ScanningOutputFormat Format = ScanningOutputFormat::Make;
+static ScanningOptimizations OptimizeArgs;
 static std::string ModuleFilesDir;
-static bool OptimizeArgs;
 static bool EagerLoadModules;
 static unsigned NumThreads = 0;
 static std::string CompilationDB;
@@ -148,10 +148,31 @@ static void ParseArgs(int argc, char **argv) {
     Format = *FormatType;
   }
 
+  std::vector<std::string> OptimizationFlags =
+      Args.getAllArgValues(OPT_optimize_args_EQ);
+  OptimizeArgs = ScanningOptimizations::None;
+  for (const auto &Arg : OptimizationFlags) {
+    auto Optimization =
+        llvm::StringSwitch<std::optional<ScanningOptimizations>>(Arg)
+            .Case("none", ScanningOptimizations::None)
+            .Case("header-search", ScanningOptimizations::HeaderSearch)
+            .Case("all", ScanningOptimizations::All)
+            .Default(std::nullopt);
+    if (!Optimization) {
+      llvm::errs()
+          << ToolName
+          << ": for the --optimize-args option: Cannot find option named '"
+          << Arg << "'\n";
+      std::exit(1);
+    }
+    OptimizeArgs |= *Optimization;
+  }
+  if (OptimizationFlags.empty())
+    OptimizeArgs = ScanningOptimizations::Default;
+
   if (const llvm::opt::Arg *A = Args.getLastArg(OPT_module_files_dir_EQ))
     ModuleFilesDir = A->getValue();
 
-  OptimizeArgs = Args.hasArg(OPT_optimize_args);
   EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
 
   if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td
@@ -18,7 +18,7 @@ defm format : Eq<"format", "The output format for the dependencies">;
 defm module_files_dir : Eq<"module-files-dir",
     "The build directory for modules. Defaults to the value of '-fmodules-cache-path=' from command lines for implicit modules">;
 
-def optimize_args : F<"optimize-args", "Whether to optimize command-line arguments of modules">;
+def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">;
 def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">;
 
 def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">;
diff --git a/llvm/include/llvm/ADT/BitmaskEnum.h b/llvm/include/llvm/ADT/BitmaskEnum.h
@@ -87,8 +87,9 @@
   using ::llvm::BitmaskEnumDetail::operator^;                                  \
   using ::llvm::BitmaskEnumDetail::operator|=;                                 \
   using ::llvm::BitmaskEnumDetail::operator&=;                                 \
+  using ::llvm::BitmaskEnumDetail::operator^=;                                 \
   /* Force a semicolon at the end of this macro. */                            \
-  using ::llvm::BitmaskEnumDetail::operator^=
+  using ::llvm::BitmaskEnumDetail::any
 
 namespace llvm {
 
@@ -136,6 +137,11 @@ constexpr unsigned bitWidth(uint64_t Value) {
   return Value ? 1 + bitWidth(Value >> 1) : 0;
 }
 
+template <typename E, typename = std::enable_if_t<is_bitmask_enum<E>::value>>
+constexpr bool any(E Val) {
+  return Val != static_cast<E>(0);
+}
+
 template <typename E, typename = std::enable_if_t<is_bitmask_enum<E>::value>>
 constexpr E operator~(E Val) {
   return static_cast<E>(~Underlying(Val) & Mask<E>());
