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

[Richard Smith]

On Mon, Jul 13, 2015 at 5:44 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> > 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?
>

Yes, I'd dug into this and reached the same conclusion myself. I'm not sure
whether LLVM expects us to be poking at its global directly, but it doesn't
make sense to both do that *and* specify the flag.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150713/8736b0b7/attachment.html>