[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

[John McCall]

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.

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

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120620/d474436b/attachment.html>