llunak created this revision.
llunak added reviewers: ABataev, rsmith, dblaikie.
llunak added projects: clang, OpenMP.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: cfe-commits.

This patch makes template instantiations be already performed in the PCH 
instead of it being done in every single file that uses the PCH. I can see 
20-30% build time saved with the few tests I've tried.

The delaying of template instantiations until the PCH is used comes from 
7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful 
reasoning for it, except for extending a unittest, which however now passes 
even with the instantiation moved back into the PCH. Since modules do not delay 
instantiation either, presumably whatever was wrong there has been fixed 
already.

I've built a complete debug build of LibreOffice with the patch and everything 
seems to work fine.

All Clang tests pass fine as well, with the exception of 23 tests, 22 of which 
are in OpenMP:

- OpenMP/declare_target_codegen.cpp fails because some virtuals of template 
classes are not emitted. This is fixed by the second change in the patch. I do 
not understand the purpose of that code (neither I have any clue about OpenMP), 
but it was introduced in d2c1dd5902782fb773c64dbf4e0b529aa4044b05 , which also 
changed this very test, and the change makes the test pass again.
- The remaining 22 tests, CodeGenCXX/vla-lambda-capturing.cpp and 21 in 
OpenMP/, all use the same FileCheck for normal compilation and using the source 
file as PCH input. Instantiating already in the PCH reorders some things, which 
makes the tests fail. Looking at the differences in the generated output, they 
all seem to be functionally equivalent to me, they just do not match exactly 
anymore.

That obviously makes the patch not ready, but I'd like to get feedback on this, 
before spending the time on adjusting the tests (which I expect will be quite a 
hassle[*]). So, is there any problem with this patch, besides adjusting the 
tests? And is there some easier way to handle those 21 OpenMP tests besides 
possibly (up to) doubling the checks in them?

[X] BTW, https://pastebin.com/Dg2L7K27 helped quite a lot with reviewing the 
differences.


Repository:
  rC Clang

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
     return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
       !isInOpenMPDeclareTargetContext() &&
       !isInOpenMPTargetExecutionDirective()) {
     if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
                                  LateParsedInstantiations.begin(),
                                  LateParsedInstantiations.end());
     LateParsedInstantiations.clear();
+
+    {
+      llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+                                     StringRef(""));
+      PerformPendingInstantiations();
+    }
   }
 
   DiagnoseUnterminatedPragmaPack();


Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
     return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
       !isInOpenMPDeclareTargetContext() &&
       !isInOpenMPTargetExecutionDirective()) {
     if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
                                  LateParsedInstantiations.begin(),
                                  LateParsedInstantiations.end());
     LateParsedInstantiations.clear();
+
+    {
+      llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+                                     StringRef(""));
+      PerformPendingInstantiations();
+    }
   }
 
   DiagnoseUnterminatedPragmaPack();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to