aganea created this revision.
aganea added reviewers: hans, thakis, rnk, erichkeane.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

A `CC1Command` was previously always burying pointers (ie. skipping object 
deletion). This lead to higher memory usage scenarios, when the inputs or 
cmd-line flags expand to more than one driver job.

With this patch, when several jobs are created by the driver, and at least one 
`CC1Command` is in the job queue, remove `-disable-free` for `CC1Command`s 
other than the last one.
If the last job is not a `CC1Command`, this patch disables `-disable-free` 
altogether for `CC1Command`s.

Fixes PR44823.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74447

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/test/Driver/cc1-spawnprocess.c
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===================================================================
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -259,6 +259,7 @@
       // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
       profilerOutput->flush();
       llvm::timeTraceProfilerCleanup();
+      Clang->clearOutputFiles(false);
     }
   }
 
Index: clang/test/Driver/cc1-spawnprocess.c
===================================================================
--- clang/test/Driver/cc1-spawnprocess.c
+++ clang/test/Driver/cc1-spawnprocess.c
@@ -20,3 +20,48 @@
 
 // YES: (in-process)
 // NO-NOT: (in-process)
+
+// The following tests ensure that only the last integrated-cc1 has -disable-free
+
+// 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
+
+// Only one TU, one job, thus no cleanup is needed.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+// CLEAN: cc1
+// CLEAN-SAME: -disable-free
+
+// Three jobs, only the last invocation will skip clean up.
+// RUN: %clang -fintegrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=NOCLEAN-LAST
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}1.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-NOT: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}2.cpp
+// NOCLEAN-LAST: cc1
+// NOCLEAN-LAST-SAME: -disable-free
+// NOCLEAN-LAST-SAME: {{.*}}3.cpp
+
+// Four jobs, no invocation will skip clean up because the last job is linking.
+// RUN: %clang -fintegrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=ALL
+// ALL-NOT: -disable-free
+
+// In no-integrated-cc1 mode, no cleanup is needed, because we're running the
+// CC1 tool in a secondary process and the process' heap will be released
+// anyway when the process ends.
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEAN
+
+// RUN: %clang -fno-integrated-cc1 -c %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}1.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}2.cpp
+// CLEANALL: cc1
+// CLEANALL-SAME: -disable-free
+// CLEANALL-SAME: {{.*}}3.cpp
+
+// RUN: %clang -fno-integrated-cc1 %t1.cpp %t2.cpp %t3.cpp -### 2>&1 | FileCheck %s --check-prefix=CLEANALL
Index: clang/lib/Driver/Job.cpp
===================================================================
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -315,6 +315,15 @@
   Environment.push_back(nullptr);
 }
 
+// In some cases when running the calling process, we need to clean-up if there
+// are other Commands being executed after us, to prevent bloating the heap.
+void Command::forceClean() {
+  auto RemoveDisableFree = [](const char *A) {
+    return StringRef(A).equals("-disable-free");
+  };
+  llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
 void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
     for (const char *Arg : InputFilenames)
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3753,6 +3753,18 @@
                        /*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // Make sure we clean-up after in-process jobs.
+  JobList &Jobs = C.getJobs();
+  if (!Jobs.empty()) {
+    Command &Last = *llvm::reverse(Jobs).begin();
+    for (auto &Job : Jobs) {
+      // If this is a in-process job, and there are other jobs coming after,
+      // then make sure we clean up once the job is done.
+      if (Job.inProcess() && &Job != &Last)
+        Job.forceClean();
+    }
+  }
+
   // If the user passed -Qunused-arguments or there were errors, don't warn
   // about any unused arguments.
   if (Diags.hasErrorOccurred() ||
Index: clang/include/clang/Driver/Job.h
===================================================================
--- clang/include/clang/Driver/Job.h
+++ clang/include/clang/Driver/Job.h
@@ -131,6 +131,12 @@
   /// Set whether to print the input filenames when executing.
   void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
 
+  /// Whether the command will be executed in this process or not.
+  virtual bool inProcess() const { return false; }
+
+  /// Prevent burying pointers, and ensure we free everything after execution.
+  void forceClean();
+
 protected:
   /// Optionally print the filenames to be compiled
   void PrintFileNames() const;
@@ -148,6 +154,8 @@
               bool *ExecutionFailed) const override;
 
   void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment) override;
+
+  bool inProcess() const override { return true; }
 };
 
 /// Like Command, but with a fallback which is executed in case
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to