[llvm] r266569 - Linker: Don't double-schedule appending variables

[Duncan P. N. Exon Smith via llvm-commits]

Author: dexonsmith
Date: Sun Apr 17 14:40:20 2016
New Revision: 266569

URL: http://llvm.org/viewvc/llvm-project?rev=266569&view=rev
Log:
Linker: Don't double-schedule appending variables

Add an assertion to ValueMapper that prevents double-scheduling of
GlobalValues to remap, and fix the one place it happened.  There are
tons of tests that fail with this assertion in place and without the
code change, so I'm not adding another.

Although it looks related, r266563 was, indeed, removing dead code.
AFAICT, this cross-file double-scheduling started in r266510 when the
cross-file recursion was removed.

Modified:
    llvm/trunk/lib/Linker/IRMover.cpp
    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp

Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=266569&r1=266568&r2=266569&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Sun Apr 17 14:40:20 2016
@@ -540,7 +540,7 @@ void IRLinker::materializeInitFor(Global
     if (!F->isDeclaration())
       return;
   } else if (auto *V = dyn_cast<GlobalVariable>(New)) {
-    if (V->hasInitializer())
+    if (V->hasInitializer() || V->hasAppendingLinkage())
       return;
   } else {
     auto *A = cast<GlobalAlias>(New);

Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=266569&r1=266568&r2=266569&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Sun Apr 17 14:40:20 2016
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/ValueMapper.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -99,6 +100,10 @@ class MDNodeMapper;
 class Mapper {
   friend class MDNodeMapper;
 
+#ifndef NDEBUG
+  DenseSet<GlobalValue *> AlreadyScheduled;
+#endif
+
   RemapFlags Flags;
   ValueMapTypeRemapper *TypeMapper;
   unsigned CurrentMCID = 0;
@@ -965,6 +970,7 @@ void Mapper::mapAppendingVariable(Global
 
 void Mapper::scheduleMapGlobalInitializer(GlobalVariable &GV, Constant &Init,
                                           unsigned MCID) {
+  assert(AlreadyScheduled.insert(&GV).second && "Should not reschedule");
   assert(MCID < MCs.size() && "Invalid mapping context");
 
   WorklistEntry WE;
@@ -980,6 +986,7 @@ void Mapper::scheduleMapAppendingVariabl
                                           bool IsOldCtorDtor,
                                           ArrayRef<Constant *> NewMembers,
                                           unsigned MCID) {
+  assert(AlreadyScheduled.insert(&GV).second && "Should not reschedule");
   assert(MCID < MCs.size() && "Invalid mapping context");
 
   WorklistEntry WE;
@@ -995,6 +1002,7 @@ void Mapper::scheduleMapAppendingVariabl
 
 void Mapper::scheduleMapGlobalAliasee(GlobalAlias &GA, Constant &Aliasee,
                                       unsigned MCID) {
+  assert(AlreadyScheduled.insert(&GA).second && "Should not reschedule");
   assert(MCID < MCs.size() && "Invalid mapping context");
 
   WorklistEntry WE;
@@ -1006,6 +1014,7 @@ void Mapper::scheduleMapGlobalAliasee(Gl
 }
 
 void Mapper::scheduleRemapFunction(Function &F, unsigned MCID) {
+  assert(AlreadyScheduled.insert(&F).second && "Should not reschedule");
   assert(MCID < MCs.size() && "Invalid mapping context");
 
   WorklistEntry WE;