[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 Tue, Jun 19, 2012 at 11:12 PM, Andrew Trick <atrick at apple.com> wrote:

> Moving to cfe-dev to catch the attention of people who know something
> about this...
>
> 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.
>

AFAICT, we've been lucky... The standard isn't (IMO) terribly clear here,
but the only reading of it I'm comfortable with is that calling a member
function of a class template requires that member function's definition to
exist, which requires instantiating it. If there *is* no definition, we've
got an invalid program.

Now, I'm not aware of any compilers that really care about this in the
cases you're dealing with -- they're usually able to call the function just
fine without instantiating it and so they carry on without a diagnostic,
and as long as we link in a definition eventually, everything "Just Works".
But I'm much more confident if we use one of the standard (C++11) or
extension (everything else) specified ways of achieving this result.

There are also some benefits to using it. If we leave any methods defined
inline (perhaps for performance reasons), we can avoid emitting them into
the '.o' file everywhere they are used but not inlined. There are a few
other similar edge cases where this actually helps.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120619/ca4fb2bc/attachment.html>