rsmith added inline comments.
================
Comment at: clang/lib/CodeGen/CGCXX.cpp:137-138
@@ -136,1 +136,4 @@
+ // Disallow aliases to available_externally because available_externally
+ // will not be there in the end to allow the creation of the alias (PR30341).
+ // FIXME: An extern template instantiation will create functions with
----------------
Instead of this, I'd just say:
> available_externally definitions aren't real definitions, so we cannot create
> an alias to one.
or similar. It doesn't seem necessary or relevant to reference a particular PR
here.
================
Comment at: clang/lib/CodeGen/CGCXX.cpp:140-142
@@ +139,5 @@
+ // FIXME: An extern template instantiation will create functions with
+ // linkage "AvailableExternally". In libc++, some classes also define
+ // members with attribute "AlwaysInline" and expect no reference to
+ // be generated. It is desirable to reenable this optimisation after
+ // corresponding LLVM changes.
----------------
It's not clear what this comment about `AlwaysInline` is referring to, since
the code does not mention that.
================
Comment at: clang/lib/CodeGen/CGCXX.cpp:142-143
@@ +141,4 @@
+ // members with attribute "AlwaysInline" and expect no reference to
+ // be generated. It is desirable to reenable this optimisation after
+ // corresponding LLVM changes.
+ if (TargetLinkage == llvm::GlobalValue::AvailableExternallyLinkage)
----------------
What "corresponding LLVM changes" are you expecting here? It seems to be
fundamental to aliases that they can only denote definitions.
================
Comment at: clang/lib/CodeGen/CGCXX.cpp:170
@@ -159,9 +169,3 @@
if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) &&
- (TargetLinkage != llvm::GlobalValue::AvailableExternallyLinkage ||
- !TargetDecl.getDecl()->hasAttr<AlwaysInlineAttr>())) {
- // FIXME: An extern template instantiation will create functions with
- // linkage "AvailableExternally". In libc++, some classes also define
- // members with attribute "AlwaysInline" and expect no reference to
- // be generated. It is desirable to reenable this optimisation after
- // corresponding LLVM changes.
+ !TargetDecl.getDecl()->hasAttr<AlwaysInlineAttr>()) {
addReplacement(MangledName, Aliasee);
----------------
Did you mean to change the behavior here? For non-`available_externally`
functions, we never used to care whether they're `always_inline`. Why do we
care now?
https://reviews.llvm.org/D24682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
