commit 34d800aa7d90e742934aa2f5f22fa731c30f4256
author Than McIntosh <thanm@google.com> Tue Jun 04 10:40:54 2019 -0400
committer Than McIntosh <thanm@google.com> Wed Jun 05 15:18:06 2019 +0000
tree ad9dbf13dcc97ab0f24b6762a0065f83d9f7216b
parent c08b8899001e26a9b58d72082b03ac343bb445e3

gollvm: adjust symbol linkage recipe for 'only_inline' functions

Change the recipe used to compute symbol linkage for functions that
the front end has marked "only_inline".  Up to this point all inline
functions emitted by the front end were marked as non-visible; now
(at the moment) all functions are marked as visible. The new recipe
is roughly:

  (!visible ? InternalLinkage :
    (only_inline ? AvailableExternallyLinkage :
                   ExternalLinkage)

Includes an update to the unit test for this attribute as well.

Change-Id: I5ebc5051ded4a9ce1bf96dcbf8ca6d074f8769ca
Reviewed-on: https://go-review.googlesource.com/c/gollvm/+/180478
Reviewed-by: Cherry Zhang <cherryyz@google.com>

bridge/go-llvm.cpp

diff --git a/bridge/go-llvm.cpp b/bridge/go-llvm.cpp
index 3e77a4f..93a5318 100644
--- a/bridge/go-llvm.cpp
+++ b/bridge/go-llvm.cpp
@@ -2580,12 +2580,20 @@
     }
   }
 
+  // At the moment don't allow the combination of only_inline and non-visible.
+  // The assumption is that if the FE exports an inlinable function, that
+  // function will be visible (in the ELF object file sense) in the host pkg.
+  assert(((flags & Backend::function_only_inline) == 0) ||
+         ((flags & Backend::function_is_visible) != 0));
+
   llvm::GlobalValue::LinkageTypes linkage =
       llvm::GlobalValue::InternalLinkage;
-  if ((flags & Backend::function_is_visible) != 0)
-    linkage = llvm::GlobalValue::ExternalLinkage;
-  else if ((flags & Backend::function_only_inline) != 0)
-    linkage = llvm::GlobalValue::AvailableExternallyLinkage;
+  if ((flags & Backend::function_is_visible) != 0) {
+    if ((flags & Backend::function_only_inline) != 0)
+      linkage = llvm::GlobalValue::AvailableExternallyLinkage;
+    else
+      linkage = llvm::GlobalValue::ExternalLinkage;
+  }
   llvm::StringRef fn(fns);
   llvm::Constant *fcnValue = nullptr;
   llvm::Value *declVal = module_->getNamedValue(fn);

unittests/BackendCore/BackendFcnTests.cpp

diff --git a/unittests/BackendCore/BackendFcnTests.cpp b/unittests/BackendCore/BackendFcnTests.cpp
index 659b0d5..8e94c3c 100644
--- a/unittests/BackendCore/BackendFcnTests.cpp
+++ b/unittests/BackendCore/BackendFcnTests.cpp
@@ -88,8 +88,8 @@
   for (auto vis : is_visible) {
     for (auto inl : is_inlinable) {
       for (auto only_inl : only_inline) {
-        // Functions imported only for inlining cannot be exported.
-        if (only_inl && vis)
+        // Assume all inlinable functions are visible.
+        if (only_inl && !vis)
           continue;
         for (auto nosplit : split_stack) {
           for (auto noret : is_noret) {
@@ -111,9 +111,9 @@
             EXPECT_FALSE(llfunc->isVarArg());
             EXPECT_EQ(llfunc->hasFnAttribute(Attribute::NoInline), !inl);
             EXPECT_EQ(llfunc->hasFnAttribute(Attribute::NoReturn), noret);
-            EXPECT_EQ(llfunc->hasExternalLinkage(), vis);
-            EXPECT_EQ(llfunc->hasInternalLinkage(), !vis && !only_inl);
-            EXPECT_EQ(llfunc->hasAvailableExternallyLinkage(), only_inl);
+            EXPECT_EQ(llfunc->hasInternalLinkage(), !vis);
+            EXPECT_EQ(llfunc->hasExternalLinkage(), vis && !only_inl);
+            EXPECT_EQ(llfunc->hasAvailableExternallyLinkage(), vis && only_inl);
             EXPECT_EQ(befcn->splitStack() == Bfunction::YesSplit, !nosplit);
           }
         }