[PATCH] D27285: llvm-lto2: Simpler workaround for PR30396.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc created this revision.
pcc added a reviewer: davide.
pcc added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

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.


https://reviews.llvm.org/D27285

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


Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===================================================================
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -109,7 +109,11 @@
 
   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 @@
         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,30 +170,19 @@
         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()) {
         llvm::errs() << argv[0] << ": missing symbol resolution for " << F
                      << ',' << 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;
Index: llvm/test/ThinLTO/X86/module_asm_glob.ll
===================================================================
--- llvm/test/ThinLTO/X86/module_asm_glob.ll
+++ llvm/test/ThinLTO/X86/module_asm_glob.ll
@@ -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 \


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27285.79825.patch
Type: text/x-patch
Size: 2998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/4ead0b8c/attachment.bin>