[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

Clang Frontend
1

Moving to cfe-dev to catch the attention of people who know something about this…

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.

-Andy

2

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.

3

Short answer: extern template is unnecessary; it is sufficient to have an explicit instantiation somewhere in the code.

Longer answer: the C++ standard requires that every used templated function be defined, either in every translation unit which uses it, or in some translation unit which subsequently contains an explicit instantiation of it. This is [temp]p6:

  A function template, member function of a class template, or static data member of a class template shall be defined in every translation unit in which it is implicitly instantiated unless the corresponding specialization is explicitly instantiated in some translation unit; no diagnostic is required.

Note that an explicit instantiation of a class template is also an explicit instantiation of its members, recursively, as long they've been defined at that point in the translation unit.

Essentially, an explicit instantiation of a class template forces that translation unit to instantiate and emit all the functions associated with that template. In LLVM terms, we emit these functions with weak_odr linkage: weak instead of linkonce because this translation unit *must* define these functions, and weak instead of external because the language does not have a guarantee that cannot be implicit instantiations elsewhere.

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. But there's nothing incorrect about what you're doing now.

John.

4

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…

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

(note, i didn’t say “explicit instantiation declaration”, just “explicit instantiation” which implies “explicit instantiation definition” in C++11 parlance I think)

5

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.

6

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.

7

I'm almost sure MSVC is fine with extern template; I didn't know the
language feature existed until about a year ago, and my experiments
seemed to indicate it worked fine in VC 10.

FWIW,
- Kim

8

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'm almost sure MSVC is fine with extern template; I didn't know the
language feature existed until about a year ago, and my experiments
seemed to indicate it worked fine in VC 10.

It doesn't work in MSVC2008.

9

Hi Nico,