Change Details

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? [*] BTW, https://pastebin.com/Dg2L7K27 helped quite a lot with reviewing the differences.

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. The only way this breaks things that I've managed to find is if a .cpp file using the PCH adds another template specialization that's not mentioned in the PCH, which is why this needs to be enabled by a flag, but PCHs often require tweaking, and in this case e.g. adding a forward declaration for the specialization seems to be very well worth it, in the rare case it happens. I've built a complete debug build of LibreOffice with the patch and everything seems to work fine. Even all Clang tests pass fine with this flag forced, except for ~22 tests which fail merely because this change reorders things differently than the tests expect, without any actual change, and one OpenMP test, for which I had a suggested fix in an earlier version of this patch, but since that version went without a reaction, as long as this change needs to be opted-in, I'll leave that to whomever cares about OpenMP.

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 eitherThe only way this breaks things that I've managed to find is if a .cpp file using the PCH adds another template specialization that's not mentioned in the PCH, which is why this needs to be enabled by a flag, but PCHs often require tweaking, and in this case e.g. adding a forward declaration for the specialization seems to be very well worth it, presumably whatever was wrong there has been fixed alreadyin the rare case it happens. I've built a complete debug build of LibreOffice with the patch and everything seems to work fine. AllEven 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 testswith this flag forced, CodeGenCXX/vla-lambda-capturing.cpp and 21 in OpenMP/except for ~22 tests which fail merely because this change reorders things differently than the tests expect, all use the same FileCheck for normal compilation and using the source file as PCH input.without any actual change, Instantiating already in the PCH reorders some thingsand one OpenMP test, for which makes the tests fail.I had a suggested fix in an earlier version of this patch, Looking at the differences in the generated outputbut since that version went without a reaction, they all seem to be functionally equivalent to meas long as this change needs to be opted-in, they just do not match exactly anymoreI'll leave that to whomever cares about OpenMP. 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? [*] BTW, https://pastebin.com/Dg2L7K27 helped quite a lot with reviewing the differences.