[WIP/LTO] Improve internalize decisions

[davide]

Summary

The existing algorithm was wrong because it internalized symbols it didn't need to.
I'm bootstrapping right now to see if it actually fixes the failure I hit. Consider this a WIP/FYI patch, I'll upload a definitive version later today. Refer to the code for comments/details.

[ruiu]

ELF/LTO.cpp

78

We don't usually use auto if the type is obvious in a very narrow (e.g. RHS) context. We also use short names for variables with narrow scope, so I'd write

uint8_t V = Body->getVisibility();
return V == STV_DEFAULT || V == STV_PROTECTED;

129–138

In both cases it does the same thing. Is this intended?

ELF/LTO.cpp

     saveBCFile(M, ".lto.opt.bc");
 }
 
+static bool isVisible(SymbolBody *Body) {
+  if (Body->MustBeInDynSym)
+    return true;
+  if (Config->Shared || Config->ExportDynamic) {
+    auto Visibility = Body->getVisibility();
+    if ((Visibility == llvm::ELF::STV_DEFAULT ||
+        Visibility == llvm::ELF::STV_PROTECTED))
+      return true;
+  }
+  return false;
+}
+
 void BitcodeCompiler::add(BitcodeFile &F) {
   std::unique_ptr<IRObjectFile> Obj =
       check(IRObjectFile::create(F.MB, Context));

     // we imported the symbols and satisfied undefined references
     // to it. We can't just change linkage here because otherwise
     // the IRMover will just rename the symbol.
-    // Shared libraries need to be handled slightly differently.
-    // For now, let's be conservative and just never internalize
-    // symbols when creating a shared library.
-    if (!Config->Shared && !Config->ExportDynamic && !B->isUsedInRegularObj())
+   if (B->isUsedInRegularObj()) {
+      // Case 1: The symbol is referenced from the outside. We can't
+      // internalize this symbol.
+      Keep.push_back(GV);
+    } else if (isVisible(B)) {
+      // Case 2: The symbol is exported to the symbol table. Maybe we
+      // can internalize the symbol. So we put in a different set
+      // and we defer the decision after the mover did its job.
+      Keep.push_back(GV);
+    }
+    else {
+      // Case 3: The symbol is neither referenced or visible.
+      // We can internalize it.
       if (!Used.count(GV))
         InternalizedSyms.insert(GV->getName());
-
-    Keep.push_back(GV);
+      Keep.push_back(GV);
+    }
   }
 
   Mover.move(Obj->takeModule(), Keep,

test/ELF/lto/internalize-exportdyn.ll

   ret void
 }
 
-define hidden void @foo() {
+define void @foo() {
   ret void
 }
 
 ; Check that _start and foo are not internalized.
 ; CHECK: define void @_start()
-; CHECK: define hidden void @foo()
+; CHECK: define void @foo()