[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
chandlerc at google.com
Wed Jun 20 00:59:46 PDT 2012
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>
>> >> On Tue, Jun 19, 2012 at 8:42 PM, Andrew Trick <atrick at apple.com>
>> >> 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
>> > 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...
More information about the cfe-dev