RFR: 8347909: Automatic precompiled.hpp inclusion [v2]

[Erik Joelsson]

On Thu, 16 Jan 2025 15:49:39 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> HotSpot uses precompiled headers to speed up non-incremental builds. The current mechanism requires all .cpp files to include precompiled.hpp as the first non-comment line. This requires HotSpot devs to think about precompiled headers when they create new files and when they need to update/manage the include sections.
>> 
>> All the compilers that we use support using precompiled headers without explicitly including the associated header in the source code. I propose that we use that capability instead. 
>> 
>> The make files changes to support this are:
>> 1) Remove share/precompiled/ from the list of include directories,
>> 2) Compiler-specific changes to support this:
>> 
>> *Windows cl*:
>> * Add the share/precompiled/ include directory when generating the PCH file.
>> * Use /FI to indicate that we should include precompiled.hpp. /Yu says that it should use a PCH for precompiled.hpp. /Fp tells the compiler that it should pick up the named precompiled header.
>> 
>> From experiments it seems like it doesn't matter what name I pass in to /FI and /Yu, they just have to be the same and the specified file doesn't even have to exist, as long as we also pass in the /Fp flag we get the benefits of the precompiled header.
>> 
>> *gcc*:
>> * Use -include to include the precompiled.hpp file. Note that the compilation command line will not have share/precompiled in the include path and that the precompiled header will still be picked up correctly.
>> 
>> *clang*:
>> * No changes needed, the -include-pch line is all that we need.
>> 
>> I've verified that these changes give the expected compilation time reductions both by comparing compilation times in our CI but also by running individual compilations of .cpp files on Linux, Mac, and Windows.
>> 
>> Note that the compiler will complain if someone now tries to include precompiled.hpp explicitly.
>> 
>> Note also that we have a section about precompiled.hpp in the HotSpot style guide. If this proposal is accepted I would like to update the style guide as a separate RFE.
>> 
>> Tested by letting GHA build and running tier1-2 in our CI pipeline.
>
> Stefan Karlsson has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Update make/common/native/CompileFile.gmk
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update make/common/native/CompileFile.gmk
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>  - Update make/common/native/CompileFile.gmk
>    
>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>

make/common/native/CompileFile.gmk line 262:

> 260:         $1_USE_PCH_FLAGS := \
> 261:             -FI$$(notdir $$($1_PRECOMPILED_HEADER)) -Fp$$($1_PCH_FILE) \
> 262:                 -Yu$$(notdir $$($1_PRECOMPILED_HEADER))

Oops I messed up here.
Suggestion:

            -FI$$(notdir $$($1_PRECOMPILED_HEADER)) -Fp$$($1_PCH_FILE) \
            -Yu$$(notdir $$($1_PRECOMPILED_HEADER))

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23146#discussion_r1918843925