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

[Douglas Gregor]

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