[cfe-dev] [llvm-commits] [llvm] r158787 - in /llvm/trunk: include/llvm/Analysis/LoopInfo.h include/llvm/Analysis/LoopInfoImpl.h lib/Analysis/LoopInfo.cpp lib/CodeGen/MachineLoopInfo.cpp

[Chandler Carruth]

On Wed, Jun 20, 2012 at 12:29 AM, John McCall <rjmccall at apple.com> wrote:

>
> On Jun 20, 2012, at 12:18 AM, Chandler Carruth wrote:
>
> On Wed, Jun 20, 2012 at 12:11 AM, John McCall <rjmccall at apple.com> wrote:
>
>> On Jun 19, 2012, at 11:12 PM, Andrew Trick wrote:
>> > On Jun 19, 2012, at 10:55 PM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>> >> On Tue, Jun 19, 2012 at 8:42 PM, Andrew Trick <atrick at apple.com>
>> wrote:
>> >> Author: atrick
>> >> Date: Tue Jun 19 22:42:09 2012
>> >> New Revision: 158787
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=158787&view=rev
>> >> Log:
>> >> Move the implementation of LoopInfo into LoopInfoImpl.h.
>> >>
>> >> The implementation only needs inclusion from LoopInfo.cpp and
>> >> MachineLoopInfo.cpp. Clients of the interface should only include the
>> >> interface. This makes the interface readable and speeds up rebuilds
>> >> after modifying the implementation.
>> >>
>> >> Technically speaking, I think you need to declare the templates as
>> 'extern' for this to be valid (
>> http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=549 is
>> the best link I have).
>> >>
>> >> This is a C++11 feature, but it is available with GCC since ~forever
>> as an extension. It often "just works" without it, but my understanding is
>> that technically it isn't valid as any old TU that uses these constructs is
>> supposed to have a definition available.
>> >>
>> >> At least for compilers that support it putting the various things like:
>> >>
>> >> __extension__ extern template class LoopInfoBase<BasicBlock, Loop>;
>> >>
>> >> Into the header would make me happier...
>> >>
>> >> I'm OK relying on compilers to make this "just work" if they don't
>> support the extension (MSVC likely, dunno), but where the do, this will
>> make sure everything works together nicely.
>> >
>> > I thought the "extern template" was purely an optimization and unlikely
>> to have any benefit with clang in this limited setting. But I'm glad you
>> pointed it out, because I was hoping someone could explain the situation to
>> me.
>> >
>> > My basis for making the change to explicit instantiation is that we've
>> been using the pattern for a year now elsewhere in LLVM and no one has
>> complained.  I didn't mess with "extern template" because I didn't want to
>> introduce something confusing and compiler-specific without a good reason.
>>
>> Short answer:  extern template is unnecessary;  it is sufficient to have
>> an explicit instantiation somewhere in the code.
>>
>
> I'm confused. You say this, but then after you're very thorough
> explanation you say:
>
>
>> An explicit instantiation declaration, e.g.
>>  extern template llvm::LoopBase<BasicBlock, Loop>;
>> is a declaration that there exists an explicit instantiation of that
>> template somewhere in the translation unit.  This is a longstanding GCC
>> feature supported both by clang (always) and MSVC (apparently since at
>> least VS2008);  it is now officially codified into C++11.  Some versions of
>> MSVC give extension warnings about it;  I believe GCC and clang do as well,
>> under -pedantic.
>>
>> Essentially, an explicit instantiation declaration permits the compiler
>> to not instantiate any function bodies associated with that template.  In
>> LLVM terms, we emit these functions as external declarations at -O0, but as
>> available_externally definitions at -O1 and above.
>>
>> So you should be able to get good compile performance with just an extern
>> template declaration and a corresponding explicit instantiation, and this
>> permits inlining/interprocedural analysis as well.
>
>
> Which seems to contradict this... and...
>
>
> I don't see the contradiction.  Andy's patch is a correct solution, but
> there is an alternative solution that uses extern template.  Both have
> their merits.
>

I misread your explanation, and didn't gather that...


>
>  But there's nothing incorrect about what you're doing now.
>>
>
> But the definitions for all the member functions are no longer visible to
> the callers of those member functions... They're only defined in one TU,
> and only in that TU do we have the explicit instantiation
>
>
> That's enough.  There is a translation unit which contains definitions for
> those member functions and explicitly instantiates them.  That satisifies
> [temp]p6:  this code is well-formed.
>

I see, sorry. I thought that was only in C++11, but it is in C++98, just
tucked away beneath the export madness where i missed it. Thanks for making
me read this paragraph again, and look at the context a bit more carefully.
=]

I still tend to prefer including the extern-template declaration on any
compiler that supports it. It seems a strict (if small) improvement over
the existing code, and relying upon this rule is a safe fallback.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120620/6f8f5925/attachment.html>