[Clang] Limit -fintegrated-cc1 to only one TU

aganea · zmodem · commit 9c9e46d786d0 · 2020-02-13T11:01:43.000+01:00
As discussed in https://reviews.llvm.org/D74447, this patch disables integrated-cc1 behavior if there's more than one job to be executed. This is meant to limit memory bloating, given that currently jobs don't clean up after execution (-disable-free is always active in cc1 mode). I see this behavior as temporary until release 10.0 ships (to ease merging of this patch), then we'll reevaluate the situation, see if D74447 makes more sense on the long term. Differential Revision: https://reviews.llvm.org/D74490 (cherry picked from commit 20f1abe)
diff --git a/clang/include/clang/Driver/Job.h b/clang/include/clang/Driver/Job.h
@@ -55,9 +55,6 @@ class Command {
   /// The list of program arguments which are inputs.
   llvm::opt::ArgStringList InputFilenames;
 
-  /// Whether to print the input filenames when executing.
-  bool PrintInputFilenames = false;
-
   /// Response file name, if this command is set to use one, or nullptr
   /// otherwise
   const char *ResponseFile = nullptr;
@@ -86,6 +83,12 @@ class Command {
   void writeResponseFile(raw_ostream &OS) const;
 
 public:
+  /// Whether to print the input filenames when executing.
+  bool PrintInputFilenames = false;
+
+  /// Whether the command will be executed in this process or not.
+  bool InProcess = false;
+
   Command(const Action &Source, const Tool &Creator, const char *Executable,
           const llvm::opt::ArgStringList &Arguments,
           ArrayRef<InputInfo> Inputs);
@@ -128,9 +131,6 @@ class Command {
   /// Print a command argument, and optionally quote it.
   static void printArg(llvm::raw_ostream &OS, StringRef Arg, bool Quote);
 
-  /// Set whether to print the input filenames when executing.
-  void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
-
 protected:
   /// Optionally print the filenames to be compiled
   void PrintFileNames() const;
@@ -139,7 +139,9 @@ class Command {
 /// Use the CC1 tool callback when available, to avoid creating a new process
 class CC1Command : public Command {
 public:
-  using Command::Command;
+  CC1Command(const Action &Source, const Tool &Creator, const char *Executable,
+             const llvm::opt::ArgStringList &Arguments,
+             ArrayRef<InputInfo> Inputs);
 
   void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
              CrashReportInfo *CrashInfo = nullptr) const override;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
@@ -3757,6 +3757,11 @@ void Driver::BuildJobs(Compilation &C) const {
                        /*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // If we have more than one job, then disable integrated-cc1 for now.
+  if (C.getJobs().size() > 1)
+    for (auto &J : C.getJobs())
+      J.InProcess = false;
+
   // If the user passed -Qunused-arguments or there were errors, don't warn
   // about any unused arguments.
   if (Diags.hasErrorOccurred() ||
diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
@@ -371,14 +371,29 @@ int Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
                                    /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
 }
 
+CC1Command::CC1Command(const Action &Source, const Tool &Creator,
+                       const char *Executable,
+                       const llvm::opt::ArgStringList &Arguments,
+                       ArrayRef<InputInfo> Inputs)
+    : Command(Source, Creator, Executable, Arguments, Inputs) {
+  InProcess = true;
+}
+
 void CC1Command::Print(raw_ostream &OS, const char *Terminator, bool Quote,
                        CrashReportInfo *CrashInfo) const {
-  OS << " (in-process)\n";
+  if (InProcess)
+    OS << " (in-process)\n";
   Command::Print(OS, Terminator, Quote, CrashInfo);
 }
 
-int CC1Command::Execute(ArrayRef<llvm::Optional<StringRef>> /*Redirects*/,
+int CC1Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
                         std::string *ErrMsg, bool *ExecutionFailed) const {
+  // FIXME: Currently, if there're more than one job, we disable
+  // -fintegrate-cc1. If we're no longer a integrated-cc1 job, fallback to
+  // out-of-process execution. See discussion in https://reviews.llvm.org/D74447
+  if (!InProcess)
+    return Command::Execute(Redirects, ErrMsg, ExecutionFailed);
+
   PrintFileNames();
 
   SmallVector<const char *, 128> Argv;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6053,7 +6053,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   if (Output.getType() == types::TY_Object &&
       Args.hasFlag(options::OPT__SLASH_showFilenames,
                    options::OPT__SLASH_showFilenames_, false)) {
-    C.getJobs().getJobs().back()->setPrintInputFilenames(true);
+    C.getJobs().getJobs().back()->PrintInputFilenames = true;
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
diff --git a/clang/test/Driver/cc1-spawnprocess.c b/clang/test/Driver/cc1-spawnprocess.c
@@ -1,22 +1,37 @@
-// RUN: %clang -fintegrated-cc1 -### %s 2>&1 | FileCheck %s --check-prefix=YES
-// RUN: %clang -fno-integrated-cc1 -### %s 2>&1 | FileCheck %s --check-prefix=NO
+// RUN: %clang -fintegrated-cc1 -c -### %s 2>&1 | FileCheck %s --check-prefix=YES
+// RUN: %clang -fno-integrated-cc1 -c -### %s 2>&1 | FileCheck %s --check-prefix=NO
 
-// RUN: %clang -fintegrated-cc1 -fno-integrated-cc1 -### %s 2>&1 \
+// RUN: %clang -fintegrated-cc1 -fno-integrated-cc1 -c -### %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=NO
-// RUN: %clang -fno-integrated-cc1 -fintegrated-cc1 -### %s 2>&1 \
+// RUN: %clang -fno-integrated-cc1 -fintegrated-cc1 -c -### %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=YES
 
-// RUN: %clang_cl -fintegrated-cc1 -### -- %s 2>&1 \
+// RUN: %clang_cl -fintegrated-cc1 -c -### -- %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=YES
-// RUN: %clang_cl -fno-integrated-cc1 -### -- %s 2>&1 \
+// RUN: %clang_cl -fno-integrated-cc1 -c -### -- %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=NO
 
 // RUN: env CCC_OVERRIDE_OPTIONS=+-fintegrated-cc1 \
-// RUN:     %clang -fintegrated-cc1 -### %s 2>&1 \
+// RUN:     %clang -fintegrated-cc1 -c -### %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=YES
 // RUN: env CCC_OVERRIDE_OPTIONS=+-fno-integrated-cc1 \
-// RUN:     %clang -fintegrated-cc1 -### %s 2>&1 \
+// RUN:     %clang -fintegrated-cc1 -c -### %s 2>&1 \
 // RUN:     | FileCheck %s --check-prefix=NO
 
 // YES: (in-process)
 // NO-NOT: (in-process)
+
+// The following tests ensure that only one integrated-cc1 is executed.
+
+// Only one TU, one job, thus integrated-cc1 is enabled.
+// RUN: %clang -fintegrated-cc1 -c %s -### 2>&1 | FileCheck %s --check-prefix=YES
+
+// Only one TU, but we're linking, two jobs, thus integrated-cc1 is disabled.
+// RUN: %clang -fintegrated-cc1 %s -### 2>&1 | FileCheck %s --check-prefix=NO
+
+// RUN: echo 'int main() { return f() + g(); }' > %t1.cpp
+// RUN: echo 'int f() { return 1; }' > %t2.cpp
+// RUN: echo 'int g() { return 2; }' > %t3.cpp
+
+// Three jobs, thus integrated-cc1 is disabled.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=NO

Original file line number	Diff line number	Diff line change
`@@ -6053,7 +6053,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,`
`6053`	`6053`	`if (Output.getType() == types::TY_Object &&`
`6054`	`6054`	`Args.hasFlag(options::OPT__SLASH_showFilenames,`
`6055`	`6055`	`options::OPT__SLASH_showFilenames_, false)) {`
`6056`		`- C.getJobs().getJobs().back()->setPrintInputFilenames(true);`
	`6056`	`+ C.getJobs().getJobs().back()->PrintInputFilenames = true;`
`6057`	`6057`	`}`
`6058`	`6058`
`6059`	`6059`	`if (Arg *A = Args.getLastArg(options::OPT_pg))`