[llvm] r288288 - llvm-lto2: Simpler workaround for PR30396.

[Peter Collingbourne via llvm-commits]

Author: pcc
Date: Wed Nov 30 17:19:05 2016
New Revision: 288288

URL: http://llvm.org/viewvc/llvm-project?rev=288288&view=rev
Log:
llvm-lto2: Simpler workaround for PR30396.

Maintain the command line resolutions as a map to a list of resolutions
rather than a single resolution, and apply the resolutions in the order
observed. This is not only simpler but allows us to test the scenario where
the two symbols have different resolutions.

Differential Revision: https://reviews.llvm.org/D27285

Modified:
    llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll
    llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp

Modified: llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll?rev=288288&r1=288287&r2=288288&view=diff
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll Wed Nov 30 17:19:05 2016
@@ -6,6 +6,7 @@
 ; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck  %s --check-prefix=NM1
 
 ; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,foo,lx \
 ; RUN:     -r=%t1.bc,foo,plx \
 ; RUN:     -r=%t1.bc,_simplefunction,pl \
 ; RUN:     -r=%t2.bc,main,plx \

Modified: llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp?rev=288288&r1=288287&r2=288288&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp (original)
+++ llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp Wed Nov 30 17:19:05 2016
@@ -109,7 +109,11 @@ int main(int argc, char **argv) {
 
   cl::ParseCommandLineOptions(argc, argv, "Resolution-based LTO test harness");
 
-  std::map<std::pair<std::string, std::string>, SymbolResolution>
+  // FIXME: Workaround PR30396 which means that a symbol can appear
+  // more than once if it is defined in module-level assembly and
+  // has a GV declaration. We allow (file, symbol) pairs to have multiple
+  // resolutions and apply them in the order observed.
+  std::map<std::pair<std::string, std::string>, std::list<SymbolResolution>>
       CommandLineResolutions;
   for (std::string R : SymbolResolutions) {
     StringRef Rest = R;
@@ -132,7 +136,7 @@ int main(int argc, char **argv) {
         llvm::errs() << "invalid character " << C << " in resolution: " << R
                      << '\n';
     }
-    CommandLineResolutions[{FileName, SymbolName}] = Res;
+    CommandLineResolutions[{FileName, SymbolName}].push_back(Res);
   }
 
   std::vector<std::unique_ptr<MemoryBuffer>> MBs;
@@ -166,12 +170,6 @@ int main(int argc, char **argv) {
         check(InputFile::create(MB->getMemBufferRef()), F);
 
     std::vector<SymbolResolution> Res;
-    // FIXME: Workaround PR30396 which means that a symbol can appear
-    // more than once if it is defined in module-level assembly and
-    // has a GV declaration. Keep track of the resolutions found in this
-    // file and remove them from the CommandLineResolutions map afterwards,
-    // so that we don't flag the second one as missing.
-    std::map<std::string, SymbolResolution> CurrentFileSymResolutions;
     for (const InputFile::Symbol &Sym : Input->symbols()) {
       auto I = CommandLineResolutions.find({F, Sym.getName()});
       if (I == CommandLineResolutions.end()) {
@@ -179,17 +177,12 @@ int main(int argc, char **argv) {
                      << ',' << Sym.getName() << '\n';
         HasErrors = true;
       } else {
-        Res.push_back(I->second);
-        CurrentFileSymResolutions[Sym.getName()] = I->second;
+        Res.push_back(I->second.front());
+        I->second.pop_front();
+        if (I->second.empty())
+          CommandLineResolutions.erase(I);
       }
     }
-    for (auto I : CurrentFileSymResolutions) {
-#ifndef NDEBUG
-      auto NumErased =
-#endif
-          CommandLineResolutions.erase({F, I.first});
-      assert(NumErased > 0);
-    }
 
     if (HasErrors)
       continue;