r241620 - Wrap clang modules and pch files in an object file container.

[Adrian Prantl]

> On Jul 13, 2015, at 1:48 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Mon, Jul 13, 2015 at 9:37 AM, Adrian Prantl <aprantl at apple.com> wrote:
> I actually removed the initialization from ObjectFilePCHContainerOperations in r241653. Is this still reproducing for you?
> 
> Yes. To reproduce, delete your module cache and build anything that will trigger an implicit module build with -ftime-report.

I could reproduce this now!

Since we’re explicitly initializing CodeGenOpts, the other options are safe from being parsed twice.
However, -time-passes is special:

In BackendUtil, we are doing this:

>   if (llvm::TimePassesIsEnabled)
>     BackendArgs.push_back("-time-passes”);
>   BackendArgs.push_back(nullptr);
>   llvm::cl::ParseCommandLineOptions(BackendArgs.size() - 1,
>                                     BackendArgs.data());

but looking at the definition of the -time-passes options, it looks to me as if this is actually a noop because all it would do is set the flag that was already set. (TimePassesIsEnabled is being initialized in CodeGenAction).

> /===----------------------------------------------------------------------===//
> // TimingInfo implementation
> 
> bool llvm::TimePassesIsEnabled = false;
> static cl::opt<bool,true>
> EnableTiming("time-passes", cl::location(TimePassesIsEnabled),
>             cl::desc("Time each pass, printing elapsed time for each on exit"));

So my suggestion is to just remove the handling of TimePassesIsEnabled from BackendUtil. I’m quite unfamiliar with the command line handling so I may be missing something. Do you agree with either of the last two sentences?

I also looked into reorganizing BackendUtil to cache the TargetMachine instead, but this turned out to be much more difficult than expected because clang modules actually want different codegen options than the main compilation unit, so I’d like to avoid this for now.

-- adrian