[cfe-commits] [PATCH][MS][Review request] - Late Microsoft template parsing

[Francois Pichet]

On Fri, Apr 22, 2011 at 12:53 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Apr 22, 2011, at 9:41 AM, Douglas Gregor wrote:
>
> On Apr 21, 2011, at 4:53 AM, Francois Pichet wrote:
>
> On Thu, Apr 14, 2011 at 11:23 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Apr 14, 2011, at 8:22 AM, Francois Pichet wrote:
>
> On Sat, Apr 9, 2011 at 1:59 PM, Francois Pichet <pichet2000 at gmail.com>
> wrote:
>
> Hi here is an updated patch
>
>
> On Sat, Mar 26, 2011 at 7:29 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> Nifty. A bunch of comments below.
>
>
> Hi here is an update patch.
>
> The main difference with this patch is that template parsing is done
> only if an instantiation is actually needed.
>
> The only problem is that my patch doesn't correctly diagnostic
> multiple definition of template code. I put a FIXME and I'll look at
> this problem later (soon).
>
> With this patch all Microsoft C++ headers parse if there is no
> instantiation requested.
> With instantiation requested there are only 3-4 remaining problems.
>
> That's a *huge* improvement. Congrats!
>
> There is a 4-5% speed improvement when doing a clang self hosting with
> delayed template parsing.
> ./configure --enable-optimized CC=/usr/local/bin/clang
> CXX=/usr/local/bin/clang++
> make CXXFLAGS=-fdelayed-template-parsing.
>
> 4-5% is quite good. That gives us a decent upper bound for what we could
> achieve when using -fdelayed-template-parsing (+ as-yet-unimplemented name
> lookup tricks) as an optimization rather than as a way to better emulate
> MSVC.
>
> On my Mac Mini this is shaving about 90 seconds out of 33 minutes for
> a complete build. The resulting binary passes all the clang test.
>
> Very cool.
> @@ -1489,6 +1494,8 @@
>    Opts.MathErrno = Args.hasArg(OPT_fmath_errno);
>    Opts.InstantiationDepth = Args.getLastArgIntValue(OPT_ftemplate_depth,
> 1024,
>                                                 Diags);
> +  Opts.DelayedTemplateParsing = Args.hasArg(OPT_fdelayed_template_parsing)
> ||
> +                                Opts.Microsoft;
>    Opts.NumLargeByValueCopy =
> Args.getLastArgIntValue(OPT_Wlarge_by_value_copy,
>                                                      0, Diags);
>    Opts.MSBitfields = Args.hasArg(OPT_mms_bitfields);
> This isn't the right place to perform the check for Microsoft mode, since
> "clang -cc1" shouldn't be making this decision. Rather, the driver should
> set the default for -fdelayed-template-parsing based on whether the user has
> requested Microsoft mode, e.g., here:
> +  // -fno-delayed-template-parsing is default.
> +  if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
> +                   options::OPT_fno_delayed_template_parsing,
> +                   false))
> +    CmdArgs.push_back("-fdelayed-template-parsing");
> That said, I really don't like tying this to -fms-extensions. It's the first
> place where we've really taken the big step from adding Microsoft-specific
> extensions to implementing an non-conforming feature specifically for
> Microsoft compatibility. It's great for users that need to parse Microsoft
> headers, but it could be harmful to users who want their C++ code relatively
> clean but also need some of the Microsoft extensions.
> Could we instead add some kind of -fms-compatibility flag, and turn on
> -fdelayed-template-parsing that way?
>
> How about taking away the "|| Opts.Microsoft" check now, and committing the
> rest of the delayed-template-parsing bit? Then we can sort out the
> -fms-extensions/-fms-compatibility stuff separately.
> Thanks for working on this, it's very, very cool!
> - Doug

ok, r130022. We can continue from there.