Add __attribute__((linkonce_odr_linkage))

[yaxunl]

Summary

Add attribute((linkonce_odr_linkage)) to allow setting LLVM linkonce_odr linkage explicitly.

[rjmccall]

Okay, first off, linkonce_odr is not an acceptable term to be introducing as an attribute name. If we need this, we can find some way to describe it that isn't a random internal compiler term.

More importantly, I don't understand how your problem is solved by linkonce_odr linkage. linkonce_odr linkage is just a form of weak linkage that allow useful transformations by the compiler (specifically: the compiler can freely drop the function if it isn't being used, and it can assume that all the implementations are semantically equivalent). If you make a library with a bunch of architecture-specific linkonce_odr implementations of a function, the linker will still just pick one randomly; it won't allow them to co-exist.

[yaxunl]

Thanks for your comments.

It works like this, e.g.

$ cat prog.ll

declare i32 @foo()

define void @use_foo() {
   %a = call i32 @foo()
  ret void
}

$ cat lib_common.ll

define linkonce_odr i32 @foo() {
  ret i32 1;
}

$ cat lib_opt.ll

define linkonce_odr i32 @foo() {
  ret i32 2;
}

$ llvm-link prog.ll lib_common.ll -override lib_opt.ll -S

; ModuleID = 'llvm-link'

define void @use_foo() {
  %a = call i32 @foo()
  ret void
}

define linkonce_odr i32 @foo() {
  ret i32 2
}

We can put all common functions in lib_common.ll, then only put a subset in lib_opt.ll. Functions in lib_opt.ll will override functions in lib_common.ll. For different GPUs we provide different lib_opt.ll. Each GPU may override different subset of lib_common.ll.

We use attribute((linkonce_odr_linkage)) by following the precedence of attribute((internal_linkage)) which exposes the LLVM internal_linkage to C/C++ programmers. We would like to accept suggestions for a better way to expose the linkonce_odr linkage.

[rjmccall]

Your use case violates the "ODR" restriction on linkonce_odr.

Do you maybe just want attribute((weak))?

[yaxunl]

My last example is not proper. In real cases, the functions are overridden by functions with the same name and semantics but optimized for speed.

Besides, we want unused library functions in lib_common.ll and lib_opt.ll to be dropped, weak attribute does not achieve that.

[rjmccall]

linkonce_odr would allow them to be dropped if unused by the library. In fact, we don't normally emit IR for functions that are linkonce and not used.

Do you actually want code in lib_common to e.g. inline the common implementation instead of calling the optimized one if present? Because that is also allowed by linkonce_odr.

[yaxunl]

Yes we want the overriding functions to be allowed to be inlined.

[rjmccall]

Well, your overriding definitions will be strong definitions. The question is whether you want to allow inning of the weak definitions, i.e. the possibly-overridden ones, and there I would assume not.

[yaxunl]

we can turn off inlining when we build lib_common.ll, then do optimization and inlining after linking with lib_common.ll and lib_opt.ll. Even if linkonce_odr allows inlining, it is still OK.

[yaxunl]

Simplify description of this attribute in AttrDocs since it causes some confusion.

[yaxunl]

If attribute((linkonce_odr_linkage)) is not a proper name for explicitly setting linkage, how about __attribute((linkage=linkonce_odr))? This can be extended to other linkages too.

[rjmccall]

You still don't actually want linkonce_odr linkage. You don't want the weak definition to be inlined, so it can't be ODR, and you want to force it to be emitted in your library, so it can't be linkonce. You just want weak linkage. There's an existing attribute for that.

At best, you want a pass on your completed library to promote any still-weak definitions to strong. That is not something we can usefully support in an attribute; it needs a custom pass.

Dropping unused definitions from your library when it's linked into an actual application is just standard dead-code stripping and does not need a special linkage.

[yaxunl]

Sorry my previous example may have caused some confusion.

Previously I said I wanted to override function definitions. However the only reason we want to add this attribute is so that unused functions will be dropped by the linker.

[rjmccall]

Does your linker not supported dead-code stripping?

[rjmccall]

You could also get this effect by somehow making the definitions linkonce_odr when they're linked in from the library. Or you could simply use the classic static-archive technique of putting each symbol in its own object file and linking against the whole thing as a static archive (.a), which only pulls in object files that provide symbols that are used.

Regardless, linkonce won't get the effect you want, because (1) linkonce definitions can be dropped at any time when they're not used, so they won't be emitted in the library if they're not used there, and (2) the overriding definitions will be strong.

[• tstellarAMD]

Hi John,

The problem we are trying to solve here is linking a LLVM bitcode program (OpenCL kernel in this specific case) with an LLVM bitcode library (OpenCL builtin library) and having the resulting LLVM bitcode module contain only the program code and the library functions that it uses.

The solution for this problem that we are using for libclc is to compile the OpenCL library to bitcode and then run a post-processing pass: https://github.com/llvm-mirror/libclc/blob/master/utils/prepare-builtins.cpp on the bitcode to set the linkage of the library functions to linkonce_odr. Doing this gives us what we are looking for, because the LLVM IR linker will drop all the linkonce_odr definitions that aren't used when it links the bitcode library in with the program.

The rationale for this specific patch was that if we could apply the linkonce_odr attribute to the functions at the source level, then we could avoid having to use the LLVM IR post-processing pass, which seems like a much cleaner and more portable solution.

Based on this comment:

You could also get this effect by somehow making the definitions linkonce_odr when they're linked in from the library.

It seems like you are suggesting that a post-processing pass like we have would be better, but I'm not sure if you had the complete picture of what we were trying to do. GIven the problem I've described above is a post-processing pass the way to go or do you have some other suggestion?

Thanks,
Tom

[rjmccall]

Yes, you should just stick with your post-processing pass or something like it.

The design of linkonce_odr linkage is that such definitions will only be emitted when they are actually used. Even with this attribute, a translation unit that consists solely of:

__attribute__((linkonce_odr_linkage)) void foo() {}

would naturally be emitted as an empty object file. It is possible that Clang as patched will not do this, because you haven't updated the places that implement lazy code emission to be aware of the new attribute. However, that strikes me as a bug, not a guaranteed aspect of the semantics of the attribute. Furthermore, LLVM module-level optimizations could also theoretically drop the function body at any time as soon as they see that it is unused.

It seems to me that the right solution is for the definitions to remain strong and then only be selectively brought in when necessary to satisfy dependencies. That could be done with a post-pass that makes the definitions linkonce_odr, or perhaps with an IR linker that simulates static-archive behavior.

include/clang/Basic/Attr.td

   let Subjects = SubjectList<[Var, Function, CXXRecord]>;
   let Documentation = [InternalLinkageDocs];
 }
+
+def LinkOnceODRLinkage : InheritableAttr {
+  let Spellings = [GNU<"linkonce_odr_linkage">];
+  let Subjects = SubjectList<[Var, Function]>;
+  let Documentation = [LinkOnceODRLinkageDocs];
+}
 No newline at end of file

include/clang/Basic/AttrDocs.td

   }];
 }
 
+def LinkOnceODRLinkageDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``linkonce_odr_linkage`` attribute changes the linkage type of the declaration to linkonce_odr.
+  }];
+}
+
 def DisableTailCallsDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

include/clang/Sema/Sema.h

                                                 unsigned AttrSpellingListIndex);
   CommonAttr *mergeCommonAttr(Decl *D, SourceRange Range, IdentifierInfo *Ident,
                               unsigned AttrSpellingListIndex);
+  LinkOnceODRLinkageAttr *
+  mergeLinkOnceODRLinkageAttr(Decl *D, SourceRange Range,
+                              IdentifierInfo *Ident,
+                              unsigned AttrSpellingListIndex);
 
   void mergeDeclAttributes(NamedDecl *New, Decl *Old,
                            AvailabilityMergeKind AMK = AMK_Redeclaration);

lib/CodeGen/CodeGenModule.cpp

 llvm::GlobalValue::LinkageTypes
 CodeGenModule::getFunctionLinkage(GlobalDecl GD) {
   const auto *D = cast<FunctionDecl>(GD.getDecl());
-
   GVALinkage Linkage = getContext().GetGVALinkageForFunction(D);
 
   if (isa<CXXDestructorDecl>(D) &&

 
 llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
     const DeclaratorDecl *D, GVALinkage Linkage, bool IsConstantVariable) {
+  if (D->hasAttr<LinkOnceODRLinkageAttr>())
+    return llvm::GlobalVariable::LinkOnceODRLinkage;
+
   if (Linkage == GVA_Internal)
     return llvm::Function::InternalLinkage;
 

lib/Sema/SemaDeclAttr.cpp

       InternalLinkageAttr(Range, Context, AttrSpellingListIndex);
 }
 
+LinkOnceODRLinkageAttr *
+Sema::mergeLinkOnceODRLinkageAttr(Decl *D, SourceRange Range,
+                                  IdentifierInfo *Ident,
+                                  unsigned AttrSpellingListIndex) {
+  if (checkAttrMutualExclusion<InternalLinkageAttr>(*this, D, Range, Ident) ||
+      checkAttrMutualExclusion<CommonAttr>(*this, D, Range, Ident))
+    return nullptr;
+
+  return ::new (Context) LinkOnceODRLinkageAttr(Range,
+                                                Context,
+                                                AttrSpellingListIndex);
+}
+
 MinSizeAttr *Sema::mergeMinSizeAttr(Decl *D, SourceRange Range,
                                     unsigned AttrSpellingListIndex) {
   if (OptimizeNoneAttr *Optnone = D->getAttr<OptimizeNoneAttr>()) {

     D->addAttr(Internal);
 }
 
+static void handleLinkOnceODRLinkageAttr(Sema &S, Decl *D,
+                                      const AttributeList &Attr) {
+  if (LinkOnceODRLinkageAttr *LinkOnce =
+          S.mergeLinkOnceODRLinkageAttr(D, Attr.getRange(), Attr.getName(),
+                                        Attr.getAttributeSpellingListIndex()))
+    D->addAttr(LinkOnce);
+}
+
 /// Handles semantic checking for features that are common to all attributes,
 /// such as checking whether a parameter was properly specified, or the correct
 /// number of arguments were passed, etc.

   case AttributeList::AT_InternalLinkage:
     handleInternalLinkageAttr(S, D, Attr);
     break;
+  case AttributeList::AT_LinkOnceODRLinkage:
+    handleLinkOnceODRLinkageAttr(S, D, Attr);
 
   // Microsoft attributes:
   case AttributeList::AT_MSNoVTable:

test/CodeGen/attr-linkonce-odr-linkage.c

+// RUN: %clang_cc1 -emit-llvm -triple i386-linux-gnu < %s | FileCheck %s
+
+// CHECK: @a = linkonce_odr global i32 0
+__attribute__((linkonce_odr_linkage)) int a;
+
+// CHECK: define linkonce_odr void @test1_g()
+void test1_g(void) __attribute__((linkonce_odr_linkage));
+

test/Sema/attr-linkonce-odr-linkage.c

+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+extern int g0 __attribute__((linkonce_odr_linkage));
+int g2 __attribute__((linkonce_odr_linkage));
+int __attribute__((linkonce_odr_linkage)) g4(void);
+void __attribute__((linkonce_odr_linkage)) g5(void) {
+}
+
+struct __attribute__((linkonce_odr_linkage)) s0 {}; // expected-warning {{'linkonce_odr_linkage' attribute only applies to variables and functions}}
+
+static int x __attribute__((linkonce_odr_linkage)); // expected-error {{weak declaration cannot have internal linkage}}
+
+int C; // expected-note {{previous definition is here}}
+extern int C __attribute__((linkonce_odr_linkage)); // expected-warning {{an already-declared variable is made a weak_import declaration}}
+
+static int pr14946_x;
+extern int pr14946_x  __attribute__((linkonce_odr_linkage)); // expected-error {{weak declaration cannot have internal linkage}}
+
+static void pr14946_f();
+void pr14946_f() __attribute__((linkonce_odr_linkage)); // expected-error {{weak declaration cannot have internal linkage}}