[flang] Use precompiled parsing headers

[mrkajetanp]

Most of the high memory usage and compilation time in the frontend units is due to including large parsing headers.
This commit moves out several of the largest parsing headers into a new precompiled header linked to flangFrontend.
The new compilation metrics for FrontendActions.cpp are as follows:

User time (seconds): 38.40
System time (seconds): 2.00
Maximum resident set size (kbytes): 2710964 (2.58 GB) (vs 3.78 GB)

ParserActions.cpp:

User time (seconds): 69.37
System time (seconds): 1.81
Maximum resident set size (kbytes): 2599456 (2.47 GB) (vs 4 GB)

Alongside the new precompiled header compilation unit

User time (seconds): 41.61
System time (seconds): 2.72
Maximum resident set size (kbytes): 3107644 (2.96 GB)

[llvmbot]

@llvm/pr-subscribers-flang-driver

Author: Kajetan Puchalski (mrkajetanp)

Changes

Most of the high memory usage and compilation time in the frontend units is due to including large parsing headers.
This commit moves out several of the largest parsing headers into a new precompiled header linked to flangFrontend.
The new compilation metrics for FrontendActions.cpp are as follows:

User time (seconds): 38.40
System time (seconds): 2.00
Maximum resident set size (kbytes): 2710964 (2.58 GB) (vs 3.78 GB)

ParserActions.cpp:

User time (seconds): 69.37
System time (seconds): 1.81
Maximum resident set size (kbytes): 2599456 (2.47 GB) (vs 4 GB)

Alongside the new precompiled header compilation unit

User time (seconds): 41.61
System time (seconds): 2.72
Maximum resident set size (kbytes): 3107644 (2.96 GB)

---

Full diff: https://github.com/llvm/llvm-project/pull/130600.diff

4 Files Affected:

• (modified) flang/lib/Frontend/CMakeLists.txt (+8)
• (modified) flang/lib/Frontend/CompilerInstance.cpp (-1)
• (modified) flang/lib/Frontend/FrontendActions.cpp (-1)
• (modified) flang/lib/Frontend/ParserActions.cpp (-4)

diff --git a/flang/lib/Frontend/CMakeLists.txt b/flang/lib/Frontend/CMakeLists.txt
index c80373799b015..e8a098613e26f 100644
--- a/flang/lib/Frontend/CMakeLists.txt
+++ b/flang/lib/Frontend/CMakeLists.txt
@@ -72,3 +72,11 @@ add_flang_library(flangFrontend
   clangBasic
   clangDriver
 )
+
+target_precompile_headers(flangFrontend PRIVATE
+  [["flang/Parser/parsing.h"]]
+  [["flang/Parser/parse-tree.h"]]
+  [["flang/Parser/dump-parse-tree.h"]]
+  [["flang/Lower/PFTBuilder.h"]]
+  [["flang/Lower/Bridge.h"]]
+)
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index b1fa32ecb4cfc..5fd3b76b6741b 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -13,7 +13,6 @@
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/CompilerInvocation.h"
 #include "flang/Frontend/TextDiagnosticPrinter.h"
-#include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "flang/Semantics/semantics.h"
 #include "flang/Support/Fortran-features.h"
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 94de376aaf7d6..cd3e206c19550 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -15,7 +15,6 @@
 #include "flang/Frontend/CompilerInvocation.h"
 #include "flang/Frontend/FrontendOptions.h"
 #include "flang/Frontend/ParserActions.h"
-#include "flang/Lower/Bridge.h"
 #include "flang/Lower/Support/Verifier.h"
 #include "flang/Optimizer/Dialect/Support/FIRContext.h"
 #include "flang/Optimizer/Dialect/Support/KindMapping.h"
diff --git a/flang/lib/Frontend/ParserActions.cpp b/flang/lib/Frontend/ParserActions.cpp
index cc7e72f696f96..7967832c80ac9 100644
--- a/flang/lib/Frontend/ParserActions.cpp
+++ b/flang/lib/Frontend/ParserActions.cpp
@@ -12,10 +12,6 @@
 
 #include "flang/Frontend/ParserActions.h"
 #include "flang/Frontend/CompilerInstance.h"
-#include "flang/Lower/Bridge.h"
-#include "flang/Lower/PFTBuilder.h"
-#include "flang/Parser/dump-parse-tree.h"
-#include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "flang/Parser/source.h"
 #include "flang/Parser/unparse.h"

[DavidTruby]

This looks good to me, it's great to see some improvement in memory usage being worked on here!

Did this have much impact on compile time as well? I see the memory usage clearly coming down in your commit message but only the new compile time numbers and not the old ones?

I'll give it a try myself when I have a sec and see how it works here.
Thanks!

[mrkajetanp]

Did this have much impact on compile time as well? I see the memory usage clearly coming down in your commit message but only the new compile time numbers and not the old ones?

It did, I just skipped over those for brevity (and because they're very similar to those included in the previous flang commit for splitting the TUs). Here are the last measurements I got:

FrontendActions.cpp:
User time (seconds): 38.40 (vs 69.35)
System time (seconds): 2.00 (vs 3.56)

ParserActions.cpp:
User time (seconds): 69.37 (vs 109.19)
System time (seconds): 1.81 (vs 3.80)

Precompiled header:
User time (seconds): 41.61
System time (seconds): 2.72

[kparzysz]

I'm not in favor of this, I think this is way too fragile. We should favor methods that are more self-explanatory (and thus harder to break unknowingly).

[tarunprabhu]

I like the idea of anything that would improve compile times, but I have no experience using pre-compiled headers and don't feel qualified to review this.

[mrkajetanp]

I'm not in favor of this, I think this is way too fragile. We should favor methods that are more self-explanatory (and thus harder to break unknowingly).

Hopefully David's suggestion should assuage the concerns here - this doesn't actually require removing any of the already existing includes. CMake can handle this completely transparently.

[kparzysz]

Hopefully David's suggestion should assuage the concerns here - this doesn't actually require removing any of the already existing includes. CMake can handle this completely transparently.

Thanks.

I often try to compile individual files by hand when I'm dealing with compilation issues to save time. Fixing these issues may require changing headers, and if I need to reconstruct the .pch file every time it happens, it would make things more complex. If the use of precompiled headers can be "hidden" inside of the build process, it should be ok.

[mrkajetanp]

I often try to compile individual files by hand when I'm dealing with compilation issues to save time. Fixing these issues may require changing headers

I gave it try, as it stands now this still works completely fine. You can just drop the pch includes from the compile command for a given TU and it'll pull in the headers normally as before. Slower to compile but won't break.
Clang is pretty good with this too, so even if you forget to drop it but change the header that was being precompiled clang will let you know that the precompiled header needs to be rebuilt because the source has changed.

[DavidTruby]

Clang is pretty good with this too, so even if you forget to drop it but change the header that was being precompiled clang will let you know that the precompiled header needs to be rebuilt because the source has changed.

FWIW gcc will do the same, and will also warn you if your flags have changed and the pch uses old ones (I think clang does the latter as well, but I haven't checked!)

[DavidTruby]

This LGTM once the other reviewers are happy!

I think you should just post the other PCH changes (for e.g. Parser and Lower) in a follow up, because we can let this one work its way through the build bots first and see if there are any issues in the simple case before expanding it.

[mrkajetanp]

This LGTM once the other reviewers are happy!

I think you should just post the other PCH changes (for e.g. Parser and Lower) in a follow up, because we can let this one work its way through the build bots first and see if there are any issues in the simple case before expanding it.

Agreed, sounds sensible :)

[tblah]

@kparzysz are you happy to proceed with the #includes not removed or would you still prefer not to see this change?

[mgorny]

Thanks! I'm not 100% sure whether it worked, since the build OOM-ed hard for me with -j3 anyway, but it didn't OOM on lib/Frontend, so perhaps it did.

For me, the worst offender is lib/Lower, with lib/Semantics going next, and then probably lib/Evaluate and lib/Optimizer/Transforms (these three OOM with -j4).

[mrkajetanp]

For me, the worst offender is lib/Lower, with lib/Semantics going next

I've got a patch giving those the precompiled header treatment too ready to go once this gets merged and nothing goes wrong with the build bots :)

[Meinersbur]

There is a problem in that it breaks some ccache builds:

• https://lab.llvm.org/staging/#/builders/206/builds/174

In any case, it renders ccache ineffective (without additional accomodations):

• https://ccache.dev/manual/latest.html#_precompiled_headers

[mrkajetanp]

Ah that's annoying, thanks for pointing it out! Is it just the MSVC builds? I'm wondering if there's some way I could reproduce this ad-hoc and I don't have access to a Windows machine. I'm doing ccache builds on AArch64 Linux and it's working fine.
Regardless, we're already using -include-pch so I think we just need to make sure we use -fno-pch-timestampand set CCACHE_SLOPPINESS appropriately?

[mrkajetanp]

How about this?
#131397