[clang] Pass -clear-ast-before-backend in Clang::ConstructJob()

[aeubanks]

Summary

This clears the memory used for the Clang AST before we run LLVM passes.

https://llvm-compile-time-tracker.com/compare.php?from=d0a5f61c4f6fccec87fd5207e3fcd9502dd59854&to=b7437fee79e04464dd968e1a29185495f3590481&stat=max-rss
shows significant memory savings with no slowdown (in fact -O0 slightly speeds up).

For more background, see
https://lists.llvm.org/pipermail/cfe-dev/2021-September/068930.html.

[aeubanks]

update

[dblaikie]

Plan is still to address the "this only works with disable free" issue? (I've some preference that be addressed before this feature is turned on by default)

& is there a flag to pass to turn this off if someone had trouble with it? (doesn't necessarily have to be, but just checking)

[aeubanks]

In D111270#3060484, @dblaikie wrote:

Plan is still to address the "this only works with disable free" issue? (I've some preference that be addressed before this feature is turned on by default)

https://reviews.llvm.org/D111767. Thanks for pushing back, this actually does help a bit more with memory savings. I was worried that we'd be doing a bunch of pointer chasing if we supported -clear-ast-before-backend without -disable-free but couldn't measure any runtime regressions aside from some nullifying tiny wins gained by this patch.

& is there a flag to pass to turn this off if someone had trouble with it? (doesn't necessarily have to be, but just checking)

Just like -disable-free doesn't have a flag, I don't want one for this at least for now. I'd rather just revert this change and fix any issues. If we come across weird issues then we may have to add a flag.

[dblaikie]

In D111270#3062802, @aeubanks wrote:

In D111270#3060484, @dblaikie wrote:

Plan is still to address the "this only works with disable free" issue? (I've some preference that be addressed before this feature is turned on by default)

https://reviews.llvm.org/D111767. Thanks for pushing back, this actually does help a bit more with memory savings. I was worried that we'd be doing a bunch of pointer chasing if we supported -clear-ast-before-backend without -disable-free but couldn't measure any runtime regressions aside from some nullifying tiny wins gained by this patch.

Great, thanks for that!

& is there a flag to pass to turn this off if someone had trouble with it? (doesn't necessarily have to be, but just checking)

Just like -disable-free doesn't have a flag, I don't want one for this at least for now. I'd rather just revert this change and fix any issues. If we come across weird issues then we may have to add a flag.

Yep, sounds good to me.

[aeubanks]

force off for the clang interpreter
I thought I ran check-clang, apparently I didn't

[aheejin]

After this patch, -print-stats started erroring out. I filed a bug at https://bugs.llvm.org/show_bug.cgi?id=52193.

clang/lib/Driver/ToolChains/Clang.cpp

void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   if (C.getDriver().embedBitcodeMarkerOnly() && !IsUsingLTO)
     CmdArgs.push_back("-fembed-bitcode=marker");
 
   // We normally speed up the clang process a bit by skipping destructors at
   // exit, but when we're generating diagnostics we can rely on some of the
   // cleanup.
   if (!C.isForDiagnostics())
     CmdArgs.push_back("-disable-free");
+  CmdArgs.push_back("-clear-ast-before-backend");
 
 #ifdef NDEBUG
   const bool IsAssertBuild = false;
 #else
   const bool IsAssertBuild = true;
 #endif
 
   // Disable the verification pass in -asserts builds.

clang/lib/Interpreter/Interpreter.cpp

Clang->setTarget(TargetInfo::CreateTargetInfo(
       Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
   if (!Clang->hasTarget())
     return llvm::createStringError(llvm::errc::not_supported,
                                    "Initialization failed. "
                                    "Target is missing");
 
   Clang->getTarget().adjust(Clang->getDiagnostics(), Clang->getLangOpts());
 
+  // Don't clear the AST before backend codegen since we do codegen multiple
+  // times, reusing the same AST.
+  Clang->getCodeGenOpts().ClearASTBeforeBackend = false;
+
   return std::move(Clang);
 }
 
 } // anonymous namespace
 
 llvm::Expected<std::unique_ptr<CompilerInstance>>
 IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {