merge main into amd-staging (llvm#1633)

Author: ronlieb

amd/comgr/src/comgr-compiler.cpp

@@ -70,6 +70,7 @@
 #include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
@@ -462,8 +463,9 @@ bool executeAssemblerImpl(AssemblerInvocation &Opts, DiagnosticsEngine &Diags,
 
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.
   if (Opts.OutputType == AssemblerInvocation::FT_Asm) {
-    MCInstPrinter *IP = TheTarget->createMCInstPrinter(
-        llvm::Triple(Opts.Triple), Opts.OutputAsmVariant, *MAI, *MCII, *MRI);
+    std::unique_ptr<MCInstPrinter> InstructionPrinter(
+      TheTarget->createMCInstPrinter(
+         llvm::Triple(Opts.Triple), Opts.OutputAsmVariant, *MAI, *MCII, *MRI));
     std::unique_ptr<MCCodeEmitter> MCE;
     std::unique_ptr<MCAsmBackend> MAB;
     if (Opts.ShowEncoding) {
@@ -472,7 +474,7 @@ bool executeAssemblerImpl(AssemblerInvocation &Opts, DiagnosticsEngine &Diags,
       MAB.reset(TheTarget->createMCAsmBackend(*STI, *MRI, Options));
     }
     auto FOut = std::make_unique<formatted_raw_ostream>(*Out);
-    Str.reset(TheTarget->createAsmStreamer(Ctx, std::move(FOut), IP,
+    Str.reset(TheTarget->createAsmStreamer(Ctx, std::move(FOut), std::move(InstructionPrinter),
                                            std::move(MCE), std::move(MAB)));
   } else if (Opts.OutputType == AssemblerInvocation::FT_Null) {
     Str.reset(createNullStreamer(Ctx));

bolt/lib/Core/BinaryContext.cpp

@@ -579,6 +579,21 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
       TrimmedSize = EntriesAsAddress->size();
   };
 
+  auto printEntryDiagnostics = [&](raw_ostream &OS,
+                                   const BinaryFunction *TargetBF) {
+    OS << "FAIL: function doesn't contain this address\n";
+    if (!TargetBF)
+      return;
+    OS << "  ! function containing this address: " << *TargetBF << '\n';
+    if (!TargetBF->isFragment())
+      return;
+    OS << "  ! is a fragment with parents: ";
+    ListSeparator LS;
+    for (BinaryFunction *Parent : TargetBF->ParentFragments)
+      OS << LS << *Parent;
+    OS << '\n';
+  };
+
   ErrorOr<const BinarySection &> Section = getSectionForAddress(Address);
   if (!Section)
     return false;
@@ -646,25 +661,8 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
 
     // Function or one of its fragments.
     const BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Value);
-    const bool DoesBelongToFunction =
-        BF.containsAddress(Value) ||
-        (TargetBF && areRelatedFragments(TargetBF, &BF));
-    if (!DoesBelongToFunction) {
-      LLVM_DEBUG({
-        if (!BF.containsAddress(Value)) {
-          dbgs() << "FAIL: function doesn't contain this address\n";
-          if (TargetBF) {
-            dbgs() << "  ! function containing this address: "
-                   << TargetBF->getPrintName() << '\n';
-            if (TargetBF->isFragment()) {
-              dbgs() << "  ! is a fragment";
-              for (BinaryFunction *Parent : TargetBF->ParentFragments)
-                dbgs() << ", parent: " << Parent->getPrintName();
-              dbgs() << '\n';
-            }
-          }
-        }
-      });
+    if (!TargetBF || !areRelatedFragments(TargetBF, &BF)) {
+      LLVM_DEBUG(printEntryDiagnostics(dbgs(), TargetBF));
       break;
     }
 
@@ -703,10 +701,7 @@ void BinaryContext::populateJumpTables() {
        ++JTI) {
     JumpTable *JT = JTI->second;
 
-    bool NonSimpleParent = false;
-    for (BinaryFunction *BF : JT->Parents)
-      NonSimpleParent |= !BF->isSimple();
-    if (NonSimpleParent)
+    if (!llvm::all_of(JT->Parents, std::mem_fn(&BinaryFunction::isSimple)))
       continue;
 
     uint64_t NextJTAddress = 0;
@@ -840,33 +835,25 @@ BinaryContext::getOrCreateJumpTable(BinaryFunction &Function, uint64_t Address,
     assert(JT->Type == Type && "jump table types have to match");
     assert(Address == JT->getAddress() && "unexpected non-empty jump table");
 
-    // Prevent associating a jump table to a specific fragment twice.
-    if (!llvm::is_contained(JT->Parents, &Function)) {
-      assert(llvm::all_of(JT->Parents,
-                          [&](const BinaryFunction *BF) {
-                            return areRelatedFragments(&Function, BF);
-                          }) &&
-             "cannot re-use jump table of a different function");
-      // Duplicate the entry for the parent function for easy access
-      JT->Parents.push_back(&Function);
-      if (opts::Verbosity > 2) {
-        this->outs() << "BOLT-INFO: Multiple fragments access same jump table: "
-                     << JT->Parents[0]->getPrintName() << "; "
-                     << Function.getPrintName() << "\n";
-        JT->print(this->outs());
-      }
-      Function.JumpTables.emplace(Address, JT);
-      for (BinaryFunction *Parent : JT->Parents)
-        Parent->setHasIndirectTargetToSplitFragment(true);
-    }
+    if (llvm::is_contained(JT->Parents, &Function))
+      return JT->getFirstLabel();
 
-    bool IsJumpTableParent = false;
-    (void)IsJumpTableParent;
-    for (BinaryFunction *Frag : JT->Parents)
-      if (Frag == &Function)
-        IsJumpTableParent = true;
-    assert(IsJumpTableParent &&
+    // Prevent associating a jump table to a specific fragment twice.
+    auto isSibling = std::bind(&BinaryContext::areRelatedFragments, this,
+                               &Function, std::placeholders::_1);
+    assert(llvm::all_of(JT->Parents, isSibling) &&
            "cannot re-use jump table of a different function");
+    if (opts::Verbosity > 2) {
+      this->outs() << "BOLT-INFO: multiple fragments access the same jump table"
+                   << ": " << *JT->Parents[0] << "; " << Function << '\n';
+      JT->print(this->outs());
+    }
+    if (JT->Parents.size() == 1)
+      JT->Parents.front()->setHasIndirectTargetToSplitFragment(true);
+    Function.setHasIndirectTargetToSplitFragment(true);
+    // Duplicate the entry for the parent function for easy access
+    JT->Parents.push_back(&Function);
+    Function.JumpTables.emplace(Address, JT);
     return JT->getFirstLabel();
   }
 

bolt/lib/Core/BinaryEmitter.cpp

@@ -809,52 +809,50 @@ void BinaryEmitter::emitJumpTable(const JumpTable &JT, MCSection *HotSection,
     Streamer.switchSection(JT.Count > 0 ? HotSection : ColdSection);
     Streamer.emitValueToAlignment(Align(JT.EntrySize));
   }
-  MCSymbol *LastLabel = nullptr;
+  MCSymbol *JTLabel = nullptr;
   uint64_t Offset = 0;
   for (MCSymbol *Entry : JT.Entries) {
     auto LI = JT.Labels.find(Offset);
-    if (LI != JT.Labels.end()) {
-      LLVM_DEBUG({
-        dbgs() << "BOLT-DEBUG: emitting jump table " << LI->second->getName()
-               << " (originally was at address 0x"
-               << Twine::utohexstr(JT.getAddress() + Offset)
-               << (Offset ? ") as part of larger jump table\n" : ")\n");
-      });
-      if (!LabelCounts.empty()) {
-        LLVM_DEBUG(dbgs() << "BOLT-DEBUG: jump table count: "
-                          << LabelCounts[LI->second] << '\n');
-        if (LabelCounts[LI->second] > 0)
-          Streamer.switchSection(HotSection);
-        else
-          Streamer.switchSection(ColdSection);
-        Streamer.emitValueToAlignment(Align(JT.EntrySize));
-      }
-      // Emit all labels registered at the address of this jump table
-      // to sync with our global symbol table.  We may have two labels
-      // registered at this address if one label was created via
-      // getOrCreateGlobalSymbol() (e.g. LEA instructions referencing
-      // this location) and another via getOrCreateJumpTable().  This
-      // creates a race where the symbols created by these two
-      // functions may or may not be the same, but they are both
-      // registered in our symbol table at the same address. By
-      // emitting them all here we make sure there is no ambiguity
-      // that depends on the order that these symbols were created, so
-      // whenever this address is referenced in the binary, it is
-      // certain to point to the jump table identified at this
-      // address.
-      if (BinaryData *BD = BC.getBinaryDataByName(LI->second->getName())) {
-        for (MCSymbol *S : BD->getSymbols())
-          Streamer.emitLabel(S);
-      } else {
-        Streamer.emitLabel(LI->second);
-      }
-      LastLabel = LI->second;
+    if (LI == JT.Labels.end())
+      goto emitEntry;
+    JTLabel = LI->second;
+    LLVM_DEBUG({
+      dbgs() << "BOLT-DEBUG: emitting jump table " << JTLabel->getName()
+             << " (originally was at address 0x"
+             << Twine::utohexstr(JT.getAddress() + Offset)
+             << (Offset ? ") as part of larger jump table\n" : ")\n");
+    });
+    if (!LabelCounts.empty()) {
+      const uint64_t JTCount = LabelCounts[JTLabel];
+      LLVM_DEBUG(dbgs() << "BOLT-DEBUG: jump table count: " << JTCount << '\n');
+      Streamer.switchSection(JTCount ? HotSection : ColdSection);
+      Streamer.emitValueToAlignment(Align(JT.EntrySize));
+    }
+    // Emit all labels registered at the address of this jump table
+    // to sync with our global symbol table.  We may have two labels
+    // registered at this address if one label was created via
+    // getOrCreateGlobalSymbol() (e.g. LEA instructions referencing
+    // this location) and another via getOrCreateJumpTable().  This
+    // creates a race where the symbols created by these two
+    // functions may or may not be the same, but they are both
+    // registered in our symbol table at the same address. By
+    // emitting them all here we make sure there is no ambiguity
+    // that depends on the order that these symbols were created, so
+    // whenever this address is referenced in the binary, it is
+    // certain to point to the jump table identified at this
+    // address.
+    if (BinaryData *BD = BC.getBinaryDataByName(JTLabel->getName())) {
+      for (MCSymbol *S : BD->getSymbols())
+        Streamer.emitLabel(S);
+    } else {
+      Streamer.emitLabel(JTLabel);
     }
+  emitEntry:
     if (JT.Type == JumpTable::JTT_NORMAL) {
       Streamer.emitSymbolValue(Entry, JT.OutputEntrySize);
     } else { // JTT_PIC
       const MCSymbolRefExpr *JTExpr =
-          MCSymbolRefExpr::create(LastLabel, Streamer.getContext());
+          MCSymbolRefExpr::create(JTLabel, Streamer.getContext());
       const MCSymbolRefExpr *E =
           MCSymbolRefExpr::create(Entry, Streamer.getContext());
       const MCBinaryExpr *Value =

bolt/lib/Passes/AsmDump.cpp

@@ -134,13 +134,14 @@ void dumpFunction(const BinaryFunction &BF) {
   std::unique_ptr<MCAsmBackend> MAB(
       BC.TheTarget->createMCAsmBackend(*BC.STI, *BC.MRI, MCTargetOptions()));
   int AsmPrinterVariant = BC.AsmInfo->getAssemblerDialect();
-  MCInstPrinter *InstructionPrinter(BC.TheTarget->createMCInstPrinter(
-      *BC.TheTriple, AsmPrinterVariant, *BC.AsmInfo, *BC.MII, *BC.MRI));
+  std::unique_ptr<MCInstPrinter> InstructionPrinter(
+      BC.TheTarget->createMCInstPrinter(*BC.TheTriple, AsmPrinterVariant,
+                                        *BC.AsmInfo, *BC.MII, *BC.MRI));
   auto FOut = std::make_unique<formatted_raw_ostream>(OS);
   FOut->SetUnbuffered();
-  std::unique_ptr<MCStreamer> AsmStreamer(
-      createAsmStreamer(*LocalCtx, std::move(FOut), InstructionPrinter,
-                        std::move(MCEInstance.MCE), std::move(MAB)));
+  std::unique_ptr<MCStreamer> AsmStreamer(createAsmStreamer(
+      *LocalCtx, std::move(FOut), std::move(InstructionPrinter),
+      std::move(MCEInstance.MCE), std::move(MAB)));
   AsmStreamer->initSections(true, *BC.STI);
   std::unique_ptr<TargetMachine> TM(BC.TheTarget->createTargetMachine(
       *BC.TheTriple, "", "", TargetOptions(), std::nullopt));

bolt/test/X86/split-func-jump-table-fragment-bidirection.s

@@ -10,7 +10,7 @@
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt -print-cfg -v=3 %t.exe -o %t.out 2>&1 | FileCheck %s
 
-# CHECK: BOLT-INFO: Multiple fragments access same jump table: main; main.cold.1
+# CHECK: BOLT-INFO: multiple fragments access the same jump table: main; main.cold.1
 # CHECK: PIC Jump table JUMP_TABLE1 for function main, main.cold.1 at {{.*}}  with a total count of 0:
 
   .text