This is an archive of the discontinued LLVM Phabricator instance.

clang-cl: Implement initial limited support for precompiled headers.
ClosedPublic

Authored by thakis on Feb 28 2016, 9:40 AM.

Details

Reviewers
hans
Summary

In the gcc precompiled header model, one explicitly runs clang with -x c++-header on a .h file to produce a gch file, and then includes the header with -include foo.h and if a .gch file exists for that header it gets used. This is documented at http://clang.llvm.org/docs/UsersManual.html#precompiled-headers

cl.exe's model is fairly different, and controlled by the two flags /Yc and /Yu. A pch file is generated as a side effect of a regular compilation when /Ycheader.h is passed. While the compilation is running, the compiler keeps track of #include lines in the main translation unit and writes everything up to an #include "header.h" line into a pch file. Conversely, /Yuheader.h tells the compiler to skip all code in the main TU up to and including #include "header.h" and instead load header.pch. (It's also possible to use /Yc and /Yu without an argument, in that case a #pragma hrdstop takes the role of controlling the point where pch ends and real code begins.)

This patch implements limited support for this in that it requires the pch header to be passed as a /FI force include flag – with this restriction, support for this can be implemented almost completely in the driver with fairly small amounts of code. For /Yu, this is trivial, and for /Yc a separate pch action is added that runs before the actual compilation. After r261774, the first failing command makes a compilation stop – this means if the pch fails to build, the main compilation won't run, which is what we want. However, in /fallback builds we need to run the main compilation even if the pch build fails, so that the main compilation's fallback can run. To achieve this, add a ForceSuccessCommand that pretends that the pch build always succeeded in /fallback builds (the main compilation will then fail to open the pch and run the fallback cl.exe invocation).

If /Yc /Yu are used in a setup that clang-cl doesn't implement yet, clang-cl will now emit a "not implemented yet, ignoring flag" warning that can be disabled by -Wno-clang-cl-pch.

Since clang-cl doesn't yet serialize some important things (most notably pragma comment(lib, ...), this feature is disabled by default and only enabled by an internal driver flag. Once it's more stable, this internal flag will disappear.

(The default stdafx.h setup passes stdafx.h as explicit argument to /Yc but not as /FI – instead every single TU has to #include <stdafx.h> as first thing it does. Implementing support for this should be possible with the approach in this patch with minimal frontend changes by passing a --stop-at / --start-at flag from the driver to the frontend. This is left for a follow-up. I don't think we ever want to support #pragma hdrstop, and supporting it with this approach isn't easy: This approach relies on the driver knowing the pch filename in advance, and #pragma hdrstop(out.pch) can set the output filename, so the driver can't know about it in advance.)

clang-cl now also honors /Fp and puts pch files in the same spot that cl.exe would put them, but the pch file format is of course incompatible. This has ramifications on /fallback, so /Yc /Yu aren't passed through to cl.exe in /fallback builds.

Diff Detail

Event Timeline

thakis updated this revision to Diff 49321.Feb 28 2016, 9:40 AM
thakis retitled this revision from to clang-cl: Implement initial limited support for precompiled headers..
thakis updated this object.
thakis added a reviewer: hans.
thakis added a subscriber: cfe-commits.
thakis updated this object.Feb 28 2016, 11:56 AM
thakis updated this revision to Diff 49325.Feb 28 2016, 11:59 AM

clean up accidentally duplicate if block

hans accepted this revision.Feb 29 2016, 12:51 PM
hans edited edge metadata.

Very nice!

I only have some style nits, really. Look forward to using this :-)

include/clang/Basic/DiagnosticDriverKinds.td
112

nit: I think it's more common in Clang's warnings to say "ignored" than "ignoring", and "flag ignored" saves one character :-)

Also I would have gone for a semicolon instead of comma, but this is not important.

Maybe "input" is redundant in "more than one input source file"?

</wordsmithing>

include/clang/Driver/CLCompatOptions.td
258

Can you add HelpText for these? I've tried to be good about documenting supported cl-mode flags.

lib/Driver/Driver.cpp
1448

A bit surprised that getValue() doesn't return a StringRef. But given that it doesn't, I suppose this is the nicest way to write it.

1466

FoundMatchingInclude?

2178

maybe add curlies around this else statement to make it a little easier to parse the else if that comes after?

2355

Maybe add .pch if needed?

lib/Driver/Tools.cpp
395

"the skip" -> just "skip"?

431

I got confused here, because even if we don't "continue", we still won't hit the else if (A->getOption().matches(options::OPT_include)) branch below.

But that branch is just dealing with implicit includes, we will still do the "Not translated, render as usual." step.

Maybe add a comment to the "else if" branch below that that code is for non-CL style PCH includes?

This revision is now accepted and ready to land.Feb 29 2016, 12:51 PM
thakis updated this revision to Diff 49541.Mar 1 2016, 1:46 PM
thakis updated this object.
thakis edited edge metadata.
thakis marked 5 inline comments as done.
  • rebase across jlebar's r261774 => implicit_inputs gone, ForceSuccessCommand is new
  • .h file is found relative to include paths => introduce -find-pch-source= cc1 flag to handle that
  • turns out several important things (#pragma comment(lib...)) aren't serialized yet => put this behind -internal-enable-pch for now so we can improve on this feature in-tree (and once it's ready that flag will go away again)
  • address view comments
lib/Driver/Driver.cpp
2355

It already does this in the next line (?)

lib/Driver/Tools.cpp
431

Done I think, but I'm not sure I understood correctly what you were asking for.

thakis updated this object.Mar 1 2016, 1:56 PM
thakis updated this revision to Diff 49544.Mar 1 2016, 1:59 PM

forgot to svn add two new test files

hans added a comment.Mar 1 2016, 3:08 PM

lgtm2

include/clang/Driver/Job.h
142 ↗(On Diff #49544)

The second sentence looks weird.

lib/Driver/Driver.cpp
2355

Not a big deal, I just meant the comment would make it more obvious that it's about adding a pch *extension* to the filename if it says ".pch" instead of "pch". I know it becomes obvious later, but then my brain had already asked "what do you mean 'add pch'"?

lib/Driver/Tools.cpp
431

You got it.

lib/Frontend/CompilerInstance.cpp
746 ↗(On Diff #49544)

Maybe add a comment about this FindPchSource mode

thakis updated this object.Mar 1 2016, 3:17 PM
thakis closed this revision.Mar 1 2016, 3:21 PM
thakis marked 3 inline comments as done.

r262420, thanks!