[BOLT] Refactor branch analysis code.

Summary:
Move the indirect branch analysis code from BinaryFunction to MCInstrAnalysis/X86MCTargetDesc.cpp.

In the process of doing this, I've added an MCRegInfo to MCInstrAnalysis which allowed me to remove a bunch of extra method parameters.  I've also had to refactor how BinaryFunction held on to instructions/offsets so that it would be easy to pass a sequence of instructions to the analysis code (rather than a map keyed by offset).

Note: I think there are a bunch of MCInstrAnalysis methods that have a BitVector output parameter that could be changed to a return value since the size of the vector is based on the number of registers, i.e. from MCRegisterInfo.  I haven't done this in order to keep the diff a more manageable size.

(cherry picked from FBD6213556)

Author: Bill Nell

bolt/BinaryFunction.cpp

@@ -461,8 +461,9 @@ class BinaryFunction {
 
   /// Temporary holder of instructions before CFG is constructed.
   /// Map offset in the function to MCInst.
-  using InstrMapType = std::map<uint32_t, MCInst>;
-  InstrMapType Instructions;
+  using InstrMapType = std::map<uint32_t, size_t>;
+  InstrMapType InstructionOffsets;
+  std::vector<MCInst> Instructions;
 
   /// List of DWARF CFI instructions. Original CFI from the binary must be
   /// sorted w.r.t. offset that it appears. We rely on this to replay CFIs
@@ -736,30 +737,25 @@ class BinaryFunction {
   }
 
   void addInstruction(uint64_t Offset, MCInst &&Instruction) {
-    Instructions.emplace(Offset, std::forward<MCInst>(Instruction));
+    assert(InstructionOffsets.size() == Instructions.size() &&
+           "There must be one instruction at every offset.");
+    Instructions.emplace_back(std::forward<MCInst>(Instruction));
+    InstructionOffsets[Offset] = Instructions.size() - 1; 
   }
 
   /// Return instruction at a given offset in the function. Valid before
   /// CFG is constructed.
   MCInst *getInstructionAtOffset(uint64_t Offset) {
     assert(CurrentState == State::Disassembled &&
            "can only call function in Disassembled state");
-    auto II = Instructions.find(Offset);
-    return (II == Instructions.end()) ? nullptr : &II->second;
+    auto II = InstructionOffsets.find(Offset);
+    return (II == InstructionOffsets.end())
+       ? nullptr : &Instructions[II->second];
   }
 
-  /// Different types of indirect branches encountered during disassembly.
-  enum class IndirectBranchType : char {
-    UNKNOWN = 0,              /// Unable to determine type.
-    POSSIBLE_TAIL_CALL,       /// Possibly a tail call.
-    POSSIBLE_JUMP_TABLE,      /// Possibly a switch/jump table.
-    POSSIBLE_PIC_JUMP_TABLE,  /// Possibly a jump table for PIC.
-    POSSIBLE_GOTO             /// Possibly a gcc's computed goto.
-  };
-
-  /// Analyze indirect branch \p Instruction before it is added to
-  /// Instructions list.
-  IndirectBranchType analyzeIndirectBranch(MCInst &Instruction,
+  /// Analyze and process indirect branch \p Instruction before it is
+  /// added to Instructions list.
+  IndirectBranchType processIndirectBranch(MCInst &Instruction,
                                            unsigned Size,
                                            uint64_t Offset);
 
@@ -1439,22 +1435,23 @@ class BinaryFunction {
     // harder for us to recover this information, since we can create empty BBs
     // with NOPs and then reorder it away.
     // We fix this by moving the CFI instruction just before any NOPs.
-    auto I = Instructions.lower_bound(Offset);
+    auto I = InstructionOffsets.lower_bound(Offset);
     if (Offset == getSize()) {
-      assert(I == Instructions.end() && "unexpected iterator value");
+      assert(I == InstructionOffsets.end() && "unexpected iterator value");
       // Sometimes compiler issues restore_state after all instructions
       // in the function (even after nop).
       --I;
       Offset = I->first;
     }
     assert(I->first == Offset && "CFI pointing to unknown instruction");
-    if (I == Instructions.begin()) {
+    if (I == InstructionOffsets.begin()) {
       CIEFrameInstructions.emplace_back(std::forward<MCCFIInstruction>(Inst));
       return;
     }
 
     --I;
-    while (I != Instructions.begin() && BC.MIA->isNoop(I->second)) {
+    while (I != InstructionOffsets.begin() &&
+           BC.MIA->isNoop(Instructions[I->second])) {
       Offset = I->first;
       --I;
     }

bolt/BinaryFunction.h

@@ -219,7 +219,7 @@ void BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
     // Create a handler entry if necessary.
     MCSymbol *LPSymbol{nullptr};
     if (LandingPad) {
-      if (Instructions.find(LandingPad) == Instructions.end()) {
+      if (InstructionOffsets.find(LandingPad) == InstructionOffsets.end()) {
         if (opts::Verbosity >= 1) {
           errs() << "BOLT-WARNING: landing pad " << Twine::utohexstr(LandingPad)
                  << " not pointing to an instruction in function "
@@ -237,11 +237,11 @@ void BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
     }
 
     // Mark all call instructions in the range.
-    auto II = Instructions.find(Start);
-    auto IE = Instructions.end();
+    auto II = InstructionOffsets.find(Start);
+    auto IE = InstructionOffsets.end();
     assert(II != IE && "exception range not pointing to an instruction");
     do {
-      auto &Instruction = II->second;
+      auto &Instruction = Instructions[II->second];
       if (BC.MIA->isCall(Instruction)) {
         assert(!BC.MIA->isInvoke(Instruction) &&
                "overlapping exception ranges detected");

bolt/Exceptions.cpp

@@ -106,10 +106,9 @@ class FrameAccessAnalysis {
     MCPhysReg Reg{0};
     int64_t StackOffset{0};
     bool IsIndexed{false};
-    if (!BC.MIA->isStackAccess(*BC.MRI, Inst, FIE.IsLoad, FIE.IsStore,
-                               FIE.IsStoreFromReg, Reg, SrcImm, FIE.StackPtrReg,
-                               StackOffset, FIE.Size, FIE.IsSimple,
-                               IsIndexed)) {
+    if (!BC.MIA->isStackAccess(Inst, FIE.IsLoad, FIE.IsStore, FIE.IsStoreFromReg,
+                               Reg, SrcImm, FIE.StackPtrReg, StackOffset, FIE.Size,
+                               FIE.IsSimple, IsIndexed)) {
       return true;
     }
 
@@ -205,7 +204,7 @@ class FrameAccessAnalysis {
       return true;
     }
 
-    if (BC.MIA->escapesVariable(Inst, *BC.MRI, SPT.HasFramePointer)) {
+    if (BC.MIA->escapesVariable(Inst, SPT.HasFramePointer)) {
       DEBUG(dbgs() << "Leaked stack address, giving up on this function.\n");
       DEBUG(dbgs() << "Blame insn: ");
       DEBUG(Inst.dump());

bolt/Passes/FrameAnalysis.cpp

@@ -44,7 +44,7 @@ class LivenessAnalysis
 
   bool isAlive(ProgramPoint PP, MCPhysReg Reg) const {
     BitVector BV = (*this->getStateAt(PP));
-    const BitVector &RegAliases = BC.MIA->getAliases(Reg, *BC.MRI);
+    const BitVector &RegAliases = BC.MIA->getAliases(Reg);
     BV &= RegAliases;
     return BV.any();
   }
@@ -60,7 +60,7 @@ class LivenessAnalysis
     BitVector BV = *this->getStateAt(P);
     BV.flip();
     BitVector GPRegs(NumRegs, false);
-    this->BC.MIA->getGPRegs(GPRegs, *this->BC.MRI);
+    this->BC.MIA->getGPRegs(GPRegs);
     BV &= GPRegs;
     int Reg = BV.find_first();
     return Reg != -1 ? Reg : 0;
@@ -91,7 +91,7 @@ class LivenessAnalysis
     // Kill
     auto Written = BitVector(NumRegs, false);
     if (!IsCall) {
-      this->BC.MIA->getWrittenRegs(Point, Written, *this->BC.MRI);
+      this->BC.MIA->getWrittenRegs(Point, Written);
     } else {
       RA.getInstClobberList(Point, Written);
       // When clobber list is conservative, it is clobbering all/most registers,
@@ -100,7 +100,7 @@ class LivenessAnalysis
       // because we don't really know what's going on.
       if (RA.isConservative(Written)) {
         Written.reset();
-        BC.MIA->getCalleeSavedRegs(Written, *this->BC.MRI);
+        BC.MIA->getCalleeSavedRegs(Written);
       }
     }
     Written.flip();

bolt/Passes/LivenessAnalysis.h

@@ -45,7 +45,7 @@ class ReachingDefOrUse
       if (Def) {
         RA.getInstClobberList(**I, BV);
       } else {
-        this->BC.MIA->getTouchedRegs(**I, BV, *this->BC.MRI);
+        this->BC.MIA->getTouchedRegs(**I, BV);
       }
       if (BV[Reg])
         return true;
@@ -101,7 +101,7 @@ class ReachingDefOrUse
     if (Def)
       RA.getInstClobberList(*Y, YClobbers);
     else
-      this->BC.MIA->getTouchedRegs(*Y, YClobbers, *this->BC.MRI);
+      this->BC.MIA->getTouchedRegs(*Y, YClobbers);
     // X kills Y if it clobbers Y completely -- this is a conservative approach.
     // In practice, we may produce use-def links that may not exist.
     XClobbers &= YClobbers;

bolt/Passes/ReachingDefOrUse.h

@@ -93,7 +93,7 @@ void RegAnalysis::beConservative(BitVector &Result) const {
     Result.set();
   } else {
     BitVector BV(BC.MRI->getNumRegs(), false);
-    BC.MIA->getCalleeSavedRegs(BV, *BC.MRI);
+    BC.MIA->getCalleeSavedRegs(BV);
     BV.flip();
     Result |= BV;
   }
@@ -104,7 +104,7 @@ bool RegAnalysis::isConservative(BitVector &Vec) const {
     return Vec.all();
   } else {
     BitVector BV(BC.MRI->getNumRegs(), false);
-    BC.MIA->getCalleeSavedRegs(BV, *BC.MRI);
+    BC.MIA->getCalleeSavedRegs(BV);
     BV |= Vec;
     return BV.all();
   }
@@ -114,9 +114,9 @@ void RegAnalysis::getInstUsedRegsList(const MCInst &Inst, BitVector &RegSet,
                                       bool GetClobbers) const {
   if (!BC.MIA->isCall(Inst)) {
     if (GetClobbers)
-      BC.MIA->getClobberedRegs(Inst, RegSet, *BC.MRI);
+      BC.MIA->getClobberedRegs(Inst, RegSet);
     else
-      BC.MIA->getUsedRegs(Inst, RegSet, *BC.MRI);
+      BC.MIA->getUsedRegs(Inst, RegSet);
     return;
   }
 

bolt/Passes/RegAnalysis.cpp

@@ -603,9 +603,9 @@ void StackLayoutModifier::performChanges() {
       bool IsStoreFromReg{false};
       uint8_t Size{0};
       bool Success{false};
-      Success = BC.MIA->isStackAccess(*BC.MRI, Inst, IsLoad, IsStore,
-                                      IsStoreFromReg, Reg, SrcImm, StackPtrReg,
-                                      StackOffset, Size, IsSimple, IsIndexed);
+      Success = BC.MIA->isStackAccess(Inst, IsLoad, IsStore, IsStoreFromReg,
+                                      Reg, SrcImm, StackPtrReg, StackOffset,
+                                      Size, IsSimple, IsIndexed);
       assert(Success && IsSimple && !IsIndexed && (!IsStore || IsStoreFromReg));
       if (StackPtrReg != BC.MIA->getFramePointer())
         Adjustment = -Adjustment;
@@ -642,13 +642,13 @@ void ShrinkWrapping::classifyCSRUses() {
                                      BitVector(DA.NumInstrs, false));
 
   const BitVector &FPAliases =
-      BC.MIA->getAliases(BC.MIA->getFramePointer(), *BC.MRI);
+      BC.MIA->getAliases(BC.MIA->getFramePointer());
   for (auto &BB : BF) {
     for (auto &Inst : BB) {
       if (BC.MIA->isCFI(Inst))
         continue;
       auto BV = BitVector(BC.MRI->getNumRegs(), false);
-      BC.MIA->getTouchedRegs(Inst, BV, *BC.MRI);
+      BC.MIA->getTouchedRegs(Inst, BV);
       BV &= CSA.CalleeSaved;
       for (int I = BV.find_first(); I != -1; I = BV.find_next(I)) {
         if (I == 0)
@@ -668,7 +668,7 @@ void ShrinkWrapping::classifyCSRUses() {
 }
 
 void ShrinkWrapping::pruneUnwantedCSRs() {
-  BitVector ParamRegs = BC.MIA->getRegsUsedAsParams(*BC.MRI);
+  BitVector ParamRegs = BC.MIA->getRegsUsedAsParams();
   for (unsigned I = 0, E = BC.MRI->getNumRegs(); I != E; ++I) {
     if (!CSA.CalleeSaved[I])
       continue;
@@ -1080,7 +1080,7 @@ bool ShrinkWrapping::doesInstUsesCSR(const MCInst &Inst, uint16_t CSR) {
       CSA.getRestoredReg(Inst) == CSR)
     return false;
   BitVector BV = BitVector(BC.MRI->getNumRegs(), false);
-  BC.MIA->getTouchedRegs(Inst, BV, *BC.MRI);
+  BC.MIA->getTouchedRegs(Inst, BV);
   return BV[CSR];
 }
 
@@ -1389,9 +1389,9 @@ void ShrinkWrapping::insertUpdatedCFI(unsigned CSR, int SPValPush,
       bool IsSimple{false};
       bool IsStoreFromReg{false};
       uint8_t Size{0};
-      if (!BC.MIA->isStackAccess(*BC.MRI, *InstIter, IsLoad, IsStore,
-                                 IsStoreFromReg, Reg, SrcImm, StackPtrReg,
-                                 StackOffset, Size, IsSimple, IsIndexed))
+      if (!BC.MIA->isStackAccess(*InstIter, IsLoad, IsStore, IsStoreFromReg,
+                                 Reg, SrcImm, StackPtrReg, StackOffset,
+                                 Size, IsSimple, IsIndexed))
         continue;
       if (Reg != CSR || !IsStore || !IsSimple)
         continue;

bolt/Passes/ShrinkWrapping.cpp

@@ -165,7 +165,7 @@ class StackPointerTrackingBase
       }
 
       if (!HasFramePointer) {
-        if (MIA->escapesVariable(Point, *this->BC.MRI, false)) {
+        if (MIA->escapesVariable(Point, false)) {
           HasFramePointer = true;
         }
       }

bolt/Passes/StackPointerTracking.h

@@ -165,8 +165,8 @@ void StokeInfo::runOnFunctions(
   DefaultDefInMask.resize(NumRegs, false);
   DefaultLiveOutMask.resize(NumRegs, false);
 
-  BC.MIA->getDefaultDefIn(DefaultDefInMask, *BC.MRI);
-  BC.MIA->getDefaultLiveOut(DefaultLiveOutMask, *BC.MRI);
+  BC.MIA->getDefaultDefIn(DefaultDefInMask);
+  BC.MIA->getDefaultLiveOut(DefaultLiveOutMask);
 
   getRegNameFromBitVec(BC, DefaultDefInMask);
   getRegNameFromBitVec(BC, DefaultLiveOutMask);