commit b35540fc2e02b23e15b0adec77db13c14c1f193a
author Cherry Zhang <cherryyz@google.com> Wed Sep 13 13:39:06 2017 -0400
committer Cherry Zhang <cherryyz@google.com> Wed Sep 13 19:30:51 2017 +0000
tree 61c508946c00f6e3a7c8a162d0a2d46bf44b793a
parent 279908bd79ae5a796bac04af8c803b79211935d7

gollvm: use internal linkage for "invisible" functions

Functions created with is_visible=false should be visible only
inside the current compilation unit. This should correspond to
LLVM's internal linkage. It seems that LLVM's hidden visibility
is for the visibility outside the shared object, not the
compilation unit.

As an example, the method wrapper for embedded type may be
defined multiple times in different packages. It has is_visible
set to false. This should not cause duplicated definition at
link time. gccgo's go.test/test/fixedbugs/issue4590.dir is
a concrete example.

Change-Id: I0c9d136fb9e0d65b071479ebec4bc97f2e42b611
Reviewed-on: https://go-review.googlesource.com/63670
Reviewed-by: Than McIntosh <thanm@google.com>

llvm-gofrontend/go-llvm.cpp

diff --git a/llvm-gofrontend/go-llvm.cpp b/llvm-gofrontend/go-llvm.cpp
index cdd828e..3509e93 100644
--- a/llvm-gofrontend/go-llvm.cpp
+++ b/llvm-gofrontend/go-llvm.cpp
@@ -2339,7 +2339,8 @@
     }
   }
 
-  llvm::GlobalValue::LinkageTypes linkage = llvm::GlobalValue::ExternalLinkage;
+  llvm::GlobalValue::LinkageTypes linkage = is_visible ?
+      llvm::GlobalValue::ExternalLinkage : llvm::GlobalValue::InternalLinkage;
   llvm::StringRef fn(fns);
   llvm::Constant *fcnValue = nullptr;
   llvm::Value *declVal = module_->getNamedValue(fn);
@@ -2367,10 +2368,6 @@
 
     fcn->addFnAttr("disable-tail-calls", "true");
 
-    // visibility
-    if (!is_visible)
-      fcn->setVisibility(llvm::GlobalValue::HiddenVisibility);
-
     // inline/noinline
     if (!is_inlinable)
       fcn->addFnAttr(llvm::Attribute::NoInline);

unittests/BackendCore/BackendFcnTests.cpp

diff --git a/unittests/BackendCore/BackendFcnTests.cpp b/unittests/BackendCore/BackendFcnTests.cpp
index 21e9465..eea8558 100644
--- a/unittests/BackendCore/BackendFcnTests.cpp
+++ b/unittests/BackendCore/BackendFcnTests.cpp
@@ -101,7 +101,8 @@
         EXPECT_EQ(llfunc->getName(), ss.str());
         EXPECT_FALSE(llfunc->isVarArg());
         EXPECT_EQ(llfunc->hasFnAttribute(Attribute::NoInline), !inl);
-        EXPECT_EQ(llfunc->hasHiddenVisibility(), !vis);
+        EXPECT_EQ(llfunc->hasExternalLinkage(), vis);
+        EXPECT_EQ(llfunc->hasInternalLinkage(), !vis);
         EXPECT_EQ(befcn->splitStack() == Bfunction::YesSplit, !split);
       }
     }
@@ -237,7 +238,7 @@
   Btype *bi64t = be->integer_type(false, 64);
   BFunctionType *befty1 = mkFuncTyp(be, L_RES, bi32t, L_PARM, bi64t, L_END);
   BFunctionType *befty2 = mkFuncTyp(be, L_RES, bi64t, L_PARM, bi64t, L_END);
-  bool is_visible = false;
+  bool is_visible = true;
   bool is_declaration = true;
   bool is_inl = true;
   bool is_splitstack = true;