Fix the third (and last known) case of code update problems due

to LLVM IR changes with addr label weirdness. In the testcase, we generate references to the two bb's when codegen'ing the first function: _test1: ## @test1 leaq Ltmp0(%rip), %rax .. leaq Ltmp1(%rip), %rax Then continue to codegen the second function where the blocks get merged. We're now smart enough to emit both labels, producing this code: _test_fun: ## @test_fun ## BB#0: ## %entry Ltmp1: ## Block address taken Ltmp0: ## BB#1: ## %ret movl $-1, %eax ret Rejoice. llvm-svn: 98595
2024-11-23 19:23:23 +01:00 · 2010-03-16 00:29:39 +00:00 · 2010-03-16 00:29:39 +00:00 · adff4d133f
commit adff4d133f
parent 241d3cb048
4 changed files with 146 additions and 28 deletions
--- a/include/llvm/CodeGen/MachineModuleInfo.h
+++ b/include/llvm/CodeGen/MachineModuleInfo.h
@ -215,6 +215,11 @@ public:
  /// because the block may be accessed outside its containing function.
  MCSymbol *getAddrLabelSymbol(const BasicBlock *BB);

+  /// getAddrLabelSymbolToEmit - Return the symbol to be used for the specified
+  /// basic block when its address is taken.  If other blocks were RAUW'd to
+  /// this one, we may have to emit them as well, return the whole set.
+  std::vector<MCSymbol*> getAddrLabelSymbolToEmit(const BasicBlock *BB);
+  
  /// takeDeletedSymbolsForFunction - If the specified function has had any
  /// references to address-taken blocks generated, but the block got deleted,
  /// return the symbol now so we can emit it.  This prevents emitting a
--- a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@ -1709,16 +1709,19 @@ void AsmPrinter::EmitBasicBlockStart(const MachineBasicBlock *MBB) const {
  if (unsigned Align = MBB->getAlignment())
    EmitAlignment(Log2_32(Align));

-  // If the block has its address taken, emit a special label to satisfy
-  // references to the block. This is done so that we don't need to
-  // remember the number of this label, and so that we can make
-  // forward references to labels without knowing what their numbers
-  // will be.
+  // If the block has its address taken, emit any labels that were used to
+  // reference the block.  It is possible that there is more than one label
+  // here, because multiple LLVM BB's may have been RAUW'd to this block after
+  // the references were generated.
  if (MBB->hasAddressTaken()) {
    const BasicBlock *BB = MBB->getBasicBlock();
    if (VerboseAsm)
-      OutStreamer.AddComment("Address Taken");
-    OutStreamer.EmitLabel(GetBlockAddressSymbol(BB));
+      OutStreamer.AddComment("Block address taken");
+    
+    std::vector<MCSymbol*> Syms = MMI->getAddrLabelSymbolToEmit(BB);
+
+    for (unsigned i = 0, e = Syms.size(); i != e; ++i)
+      OutStreamer.EmitLabel(Syms[i]);
  }

  // Print the main label for the block.
--- a/lib/CodeGen/MachineModuleInfo.cpp
+++ b/lib/CodeGen/MachineModuleInfo.cpp
@ -23,6 +23,7 @@
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/ADT/PointerUnion.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/ErrorHandling.h"
 using namespace llvm;
@ -52,7 +53,10 @@ public:
 class MMIAddrLabelMap {
  MCContext &Context;
  struct AddrLabelSymEntry {
-    MCSymbol *Sym;  // The symbol for the label.
+    /// Symbols - The symbols for the label.  This is a pointer union that is
+    /// either one symbol (the common case) or a list of symbols.
+    PointerUnion<MCSymbol *, std::vector<MCSymbol*>*> Symbols;
+    
    Function *Fn;   // The containing function of the BasicBlock.
    unsigned Index; // The index in BBCallbacks for the BasicBlock.
  };
@ -75,9 +79,17 @@ public:
  ~MMIAddrLabelMap() {
    assert(DeletedAddrLabelsNeedingEmission.empty() &&
           "Some labels for deleted blocks never got emitted");
+    
+    // Deallocate any of the 'list of symbols' case.
+    for (DenseMap<AssertingVH<BasicBlock>, AddrLabelSymEntry>::iterator
+         I = AddrLabelSymbols.begin(), E = AddrLabelSymbols.end(); I != E; ++I)
+      if (I->second.Symbols.is<std::vector<MCSymbol*>*>())
+        delete I->second.Symbols.get<std::vector<MCSymbol*>*>();
  }
  
-  MCSymbol *getAddrLabelSymbol(BasicBlock *BB);  
+  MCSymbol *getAddrLabelSymbol(BasicBlock *BB);
+  std::vector<MCSymbol*> getAddrLabelSymbolToEmit(BasicBlock *BB);
+
  void takeDeletedSymbolsForFunction(Function *F, 
                                     std::vector<MCSymbol*> &Result);

@ -92,9 +104,11 @@ MCSymbol *MMIAddrLabelMap::getAddrLabelSymbol(BasicBlock *BB) {
  AddrLabelSymEntry &Entry = AddrLabelSymbols[BB];
  
  // If we already had an entry for this block, just return it.
-  if (Entry.Sym) {
+  if (!Entry.Symbols.isNull()) {
    assert(BB->getParent() == Entry.Fn && "Parent changed");
-    return Entry.Sym;
+    if (Entry.Symbols.is<MCSymbol*>())
+      return Entry.Symbols.get<MCSymbol*>();
+    return (*Entry.Symbols.get<std::vector<MCSymbol*>*>())[0];
  }
  
  // Otherwise, this is a new entry, create a new symbol for it and add an
@ -103,9 +117,30 @@ MCSymbol *MMIAddrLabelMap::getAddrLabelSymbol(BasicBlock *BB) {
  BBCallbacks.back().setMap(this);
  Entry.Index = BBCallbacks.size()-1;
  Entry.Fn = BB->getParent();
-  return Entry.Sym = Context.CreateTempSymbol();
+  MCSymbol *Result = Context.CreateTempSymbol();
+  Entry.Symbols = Result;
+  return Result;
 }

+std::vector<MCSymbol*>
+MMIAddrLabelMap::getAddrLabelSymbolToEmit(BasicBlock *BB) {
+  assert(BB->hasAddressTaken() &&
+         "Shouldn't get label for block without address taken");
+  AddrLabelSymEntry &Entry = AddrLabelSymbols[BB];
+  
+  std::vector<MCSymbol*> Result;
+  
+  // If we already had an entry for this block, just return it.
+  if (Entry.Symbols.isNull())
+    Result.push_back(getAddrLabelSymbol(BB));
+  else if (MCSymbol *Sym = Entry.Symbols.dyn_cast<MCSymbol*>())
+    Result.push_back(Sym);
+  else
+    Result = *Entry.Symbols.get<std::vector<MCSymbol*>*>();
+  return Result;
+}
+
+
 /// takeDeletedSymbolsForFunction - If we have any deleted symbols for F, return
 /// them.
 void MMIAddrLabelMap::
@ -128,36 +163,80 @@ void MMIAddrLabelMap::UpdateForDeletedBlock(BasicBlock *BB) {
  // queue it up for later emission when the function is output.
  AddrLabelSymEntry Entry = AddrLabelSymbols[BB];
  AddrLabelSymbols.erase(BB);
-  assert(Entry.Sym && "Didn't have a symbol, why a callback?");
+  assert(!Entry.Symbols.isNull() && "Didn't have a symbol, why a callback?");
  BBCallbacks[Entry.Index] = 0;  // Clear the callback.

-  if (Entry.Sym->isDefined())
-    return;
-  
-  // If the block is not yet defined, we need to emit it at the end of the
-  // function.  Add the symbol to the DeletedAddrLabelsNeedingEmission list for
-  // the containing Function.  Since the block is being deleted, its parent may
-  // already be removed, we have to get the function from 'Entry'.
  assert((BB->getParent() == 0 || BB->getParent() == Entry.Fn) &&
         "Block/parent mismatch");
+
+  // Handle both the single and the multiple symbols cases.
+  if (MCSymbol *Sym = Entry.Symbols.dyn_cast<MCSymbol*>()) {
+    if (Sym->isDefined())
+      return;
  
-  DeletedAddrLabelsNeedingEmission[Entry.Fn].push_back(Entry.Sym);
+    // If the block is not yet defined, we need to emit it at the end of the
+    // function.  Add the symbol to the DeletedAddrLabelsNeedingEmission list
+    // for the containing Function.  Since the block is being deleted, its
+    // parent may already be removed, we have to get the function from 'Entry'.
+    DeletedAddrLabelsNeedingEmission[Entry.Fn].push_back(Sym);
+  } else {
+    std::vector<MCSymbol*> *Syms = Entry.Symbols.get<std::vector<MCSymbol*>*>();
+
+    for (unsigned i = 0, e = Syms->size(); i != e; ++i) {
+      MCSymbol *Sym = (*Syms)[i];
+      if (Sym->isDefined()) continue;  // Ignore already emitted labels.
+      
+      // If the block is not yet defined, we need to emit it at the end of the
+      // function.  Add the symbol to the DeletedAddrLabelsNeedingEmission list
+      // for the containing Function.  Since the block is being deleted, its
+      // parent may already be removed, we have to get the function from
+      // 'Entry'.
+      DeletedAddrLabelsNeedingEmission[Entry.Fn].push_back(Sym);
+    }
+    
+    // The entry is deleted, free the memory associated with the symbol list.
+    delete Syms;
+  }
 }

 void MMIAddrLabelMap::UpdateForRAUWBlock(BasicBlock *Old, BasicBlock *New) {
  // Get the entry for the RAUW'd block and remove it from our map.
  AddrLabelSymEntry OldEntry = AddrLabelSymbols[Old];
  AddrLabelSymbols.erase(Old);
-  assert(OldEntry.Sym && "Didn't have a symbol, why a callback?");
-  
+  assert(!OldEntry.Symbols.isNull() && "Didn't have a symbol, why a callback?");
+
+  AddrLabelSymEntry &NewEntry = AddrLabelSymbols[New];
+
  // If New is not address taken, just move our symbol over to it.
-  if (!AddrLabelSymbols.count(New)) {
+  if (NewEntry.Symbols.isNull()) {
    BBCallbacks[OldEntry.Index] = New;    // Update the callback.
-    AddrLabelSymbols[New] = OldEntry;     // Set New's entry.
-  } else {
-    assert(0 && "Case not handled yet!");
-    abort();
+    NewEntry = OldEntry;     // Set New's entry.
+    return;
  }
+
+  BBCallbacks[OldEntry.Index] = 0;    // Update the callback.
+
+  // Otherwise, we need to add the old symbol to the new block's set.  If it is
+  // just a single entry, upgrade it to a symbol list.
+  if (MCSymbol *PrevSym = NewEntry.Symbols.dyn_cast<MCSymbol*>()) {
+    std::vector<MCSymbol*> *SymList = new std::vector<MCSymbol*>();
+    SymList->push_back(PrevSym);
+    NewEntry.Symbols = SymList;
+  }
+      
+  std::vector<MCSymbol*> *SymList =
+    NewEntry.Symbols.get<std::vector<MCSymbol*>*>();
+
+  // If the old entry was a single symbol, add it.
+  if (MCSymbol *Sym = OldEntry.Symbols.dyn_cast<MCSymbol*>()) {
+    SymList->push_back(Sym);
+    return;
+  }
+  
+  // Otherwise, concatenate the list.
+  std::vector<MCSymbol*> *Syms =OldEntry.Symbols.get<std::vector<MCSymbol*>*>();
+  SymList->insert(SymList->end(), Syms->begin(), Syms->end());
+  delete Syms;
 }


@ -260,6 +339,18 @@ MCSymbol *MachineModuleInfo::getAddrLabelSymbol(const BasicBlock *BB) {
  return AddrLabelSymbols->getAddrLabelSymbol(const_cast<BasicBlock*>(BB));
 }

+/// getAddrLabelSymbolToEmit - Return the symbol to be used for the specified
+/// basic block when its address is taken.  If other blocks were RAUW'd to
+/// this one, we may have to emit them as well, return the whole set.
+std::vector<MCSymbol*> MachineModuleInfo::
+getAddrLabelSymbolToEmit(const BasicBlock *BB) {
+  // Lazily create AddrLabelSymbols.
+  if (AddrLabelSymbols == 0)
+    AddrLabelSymbols = new MMIAddrLabelMap(Context);
+ return AddrLabelSymbols->getAddrLabelSymbolToEmit(const_cast<BasicBlock*>(BB));
+}
+
+
 /// takeDeletedSymbolsForFunction - If the specified function has had any
 /// references to address-taken blocks generated, but the block got deleted,
 /// return the symbol now so we can emit it.  This prevents emitting a
--- a/test/CodeGen/Generic/addr-label.ll
+++ b/test/CodeGen/Generic/addr-label.ll
@ -37,3 +37,22 @@ test_label:
 ret:
        ret i32 -1
 }
+
+; Issues with a BB that gets RAUW'd to another one after references are
+; generated.
+define void @test3(i8** %P, i8** %Q) nounwind {
+entry:
+  store i8* blockaddress(@test3b, %test_label), i8** %P
+  store i8* blockaddress(@test3b, %ret), i8** %Q
+  ret void
+}
+
+define i32 @test3b() nounwind {
+entry:
+	br label %test_label
+test_label:
+	br label %ret
+ret:
+	ret i32 -1
+}
+