[BOLT][NFC] Add MCPlusBuilder defOperands/useOperands helpers

[Amir]

Summary

Make intent more explicit with the use of new helper methods.

[maksfb]

Nice!

bolt/include/bolt/Core/MCPlusBuilder.h

void moveAnnotations(MCInst &&SrcInst, MCInst &DstInst) const {
     if (!AnnotationInst)
       return;
 
     DstInst.addOperand(MCOperand::createInst(AnnotationInst));
     removeAnnotationInst(SrcInst);
   }
 
 public:
+  /// Return iterator range covering def operands.
+  iterator_range<MCInst::iterator> defOperands(MCInst &Inst) const {
+    return make_range(Inst.begin(),
+                      Inst.begin() + Info->get(Inst.getOpcode()).getNumDefs());
+  }
+
+  iterator_range<MCInst::const_iterator> defOperands(const MCInst &Inst) const {
+    return make_range(Inst.begin(),
+                      Inst.begin() + Info->get(Inst.getOpcode()).getNumDefs());
+  }
+
+  /// Return iterator range covering prime use operands.
+  iterator_range<MCInst::iterator> useOperands(MCInst &Inst) const {
+    return make_range(Inst.begin() + Info->get(Inst.getOpcode()).getNumDefs(),
+                      Inst.begin() + MCPlus::getNumPrimeOperands(Inst));
+  }
+
+  iterator_range<MCInst::const_iterator> useOperands(const MCInst &Inst) const {
+    return make_range(Inst.begin() + Info->get(Inst.getOpcode()).getNumDefs(),
+                      Inst.begin() + MCPlus::getNumPrimeOperands(Inst));
+  }
+
+public:
   class InstructionIterator {
   public:
     using iterator_category = std::bidirectional_iterator_tag;
     using value_type = MCInst;
     using difference_type = std::ptrdiff_t;
     using pointer = value_type *;
     using reference = value_type &;
 

bolt/include/bolt/Passes/LivenessAnalysis.h

if (!this->BC.MIB->isCFI(Point)) {
       if (IsCall) {
         RA.getInstUsedRegsList(Point, Used, /*GetClobbers*/ true);
         if (RA.isConservative(Used)) {
           Used = BC.MIB->getRegsUsedAsParams();
           BC.MIB->getDefaultLiveOut(Used);
         }
       }
       const MCInstrDesc &InstInfo = BC.MII->get(Point.getOpcode());
-      for (unsigned I = 0, E = Point.getNumOperands(); I != E; ++I) {
-        if (!Point.getOperand(I).isReg() || I < InstInfo.getNumDefs())
-          continue;
-        Used |= BC.MIB->getAliases(Point.getOperand(I).getReg(),
-                                   /*OnlySmaller=*/false);
-      }
+      for (const MCOperand &Op : BC.MIB->useOperands(Point))
+        if (Op.isReg())
+          Used |= BC.MIB->getAliases(Op.getReg(), /*OnlySmaller=*/false);
       for (MCPhysReg ImplicitUse : InstInfo.implicit_uses())
         Used |= BC.MIB->getAliases(ImplicitUse, false);
       if (IsCall &&
           (!BC.MIB->isTailCall(Point) || !BC.MIB->isConditionalBranch(Point))) {
         // Never gen FLAGS from a non-conditional call... this is overly
         // conservative
         Used.reset(BC.MIB->getFlagsReg());
       }

bolt/lib/Core/MCPlusBuilder.cpp

void MCPlusBuilder::getClobberedRegs(const MCInst &Inst,
   if (isPrefix(Inst) || isCFI(Inst))
     return;
 
   const MCInstrDesc &InstInfo = Info->get(Inst.getOpcode());
 
   for (MCPhysReg ImplicitDef : InstInfo.implicit_defs())
     Regs |= getAliases(ImplicitDef, /*OnlySmaller=*/false);
 
-  for (unsigned I = 0, E = InstInfo.getNumDefs(); I != E; ++I) {
-    const MCOperand &Operand = Inst.getOperand(I);
+  for (const MCOperand &Operand : defOperands(Inst)) {
     assert(Operand.isReg());
     Regs |= getAliases(Operand.getReg(), /*OnlySmaller=*/false);
   }
 }
 
 void MCPlusBuilder::getTouchedRegs(const MCInst &Inst, BitVector &Regs) const {
   if (isPrefix(Inst) || isCFI(Inst))
     return;
void MCPlusBuilder::getWrittenRegs(const MCInst &Inst, BitVector &Regs) const {
   if (isPrefix(Inst) || isCFI(Inst))
     return;
 
   const MCInstrDesc &InstInfo = Info->get(Inst.getOpcode());
 
   for (MCPhysReg ImplicitDef : InstInfo.implicit_defs())
     Regs |= getAliases(ImplicitDef, /*OnlySmaller=*/true);
 
-  for (unsigned I = 0, E = InstInfo.getNumDefs(); I != E; ++I) {
-    const MCOperand &Operand = Inst.getOperand(I);
+  for (const MCOperand &Operand : defOperands(Inst)) {
     assert(Operand.isReg());
     Regs |= getAliases(Operand.getReg(), /*OnlySmaller=*/true);
   }
 }
 
 void MCPlusBuilder::getUsedRegs(const MCInst &Inst, BitVector &Regs) const {
   if (isPrefix(Inst) || isCFI(Inst))
     return;
void MCPlusBuilder::getSrcRegs(const MCInst &Inst, BitVector &Regs) const {
   if (isRep(Inst))
     getRepRegs(Regs);
 
   const MCInstrDesc &InstInfo = Info->get(Inst.getOpcode());
 
   for (MCPhysReg ImplicitUse : InstInfo.implicit_uses())
     Regs |= getAliases(ImplicitUse, /*OnlySmaller=*/true);
 
-  for (unsigned I = InstInfo.getNumDefs(), E = InstInfo.getNumOperands();
-       I != E; ++I) {
-    if (!Inst.getOperand(I).isReg())
-      continue;
-    Regs |= getAliases(Inst.getOperand(I).getReg(), /*OnlySmaller=*/true);
-  }
+  for (const MCOperand &Operand : useOperands(Inst))
+    if (Operand.isReg())
+      Regs |= getAliases(Operand.getReg(), /*OnlySmaller=*/true);
 }
 
 bool MCPlusBuilder::hasDefOfPhysReg(const MCInst &MI, unsigned Reg) const {
   const MCInstrDesc &InstInfo = Info->get(MI.getOpcode());
   return InstInfo.hasDefOfPhysReg(MI, Reg, *RegInfo);
 }
 
 bool MCPlusBuilder::hasUseOfPhysReg(const MCInst &MI, unsigned Reg) const {

bolt/lib/Passes/ShrinkWrapping.cpp

 
 void StackLayoutModifier::checkStackPointerRestore(MCInst &Point) {
   StackPointerTracking &SPT = Info.getStackPointerTracking();
   if (!BC.MII->get(Point.getOpcode())
            .hasDefOfPhysReg(Point, BC.MIB->getStackPointer(), *BC.MRI))
     return;
   // Check if the definition of SP comes from FP -- in this case, this
   // value may need to be updated depending on our stack layout changes
-  const MCInstrDesc &InstInfo = BC.MII->get(Point.getOpcode());
-  unsigned NumDefs = InstInfo.getNumDefs();
-  bool UsesFP = llvm::any_of(
-      llvm::drop_begin(MCPlus::primeOperands(Point), NumDefs),
-      [&](MCOperand &Op) {
-        return Op.isReg() && Op.getReg() == BC.MIB->getFramePointer();
-      });
+  bool UsesFP = llvm::any_of(BC.MIB->useOperands(Point), [&](MCOperand &Op) {
   if (!UsesFP)
     return;
 
   // Setting up evaluation
   int SPVal, FPVal;
   std::tie(SPVal, FPVal) = *SPT.getStateBefore(Point);
   std::pair<MCPhysReg, int64_t> FP;
 

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

public:
 
   bool isLoad(const MCInst &Inst) const override {
     return isLDRB(Inst) || isLDRH(Inst) || isLDRW(Inst) || isLDRX(Inst);
   }
 
   bool isLoadFromStack(const MCInst &Inst) const {
     if (!isLoad(Inst))
       return false;
-    const MCInstrDesc &InstInfo = Info->get(Inst.getOpcode());
-    unsigned NumDefs = InstInfo.getNumDefs();
-    for (unsigned I = NumDefs, E = InstInfo.getNumOperands(); I < E; ++I) {
-      const MCOperand &Operand = Inst.getOperand(I);
+    for (const MCOperand &Operand : useOperands(Inst)) {
       if (!Operand.isReg())
         continue;
       unsigned Reg = Operand.getReg();
       if (Reg == AArch64::SP || Reg == AArch64::WSP || Reg == AArch64::FP ||
           Reg == AArch64::W29)
         return true;
     }
     return false;

bolt/lib/Target/X86/X86MCPlusBuilder.cpp

bool isStackAdjustment(const MCInst &Inst) const override {
     case X86::SUB64ri32:
     case X86::SUB64ri8:
     case X86::ADD64ri32:
     case X86::ADD64ri8:
     case X86::LEA64r:
       break;
     }
 
-    const MCInstrDesc &MCII = Info->get(Inst.getOpcode());
-    for (int I = 0, E = MCII.getNumDefs(); I != E; ++I) {
-      const MCOperand &Operand = Inst.getOperand(I);
-      if (Operand.isReg() && Operand.getReg() == X86::RSP)
-        return true;
-    }
-    return false;
+    return any_of(defOperands(Inst), [](const MCOperand &Op) {
+      return Op.isReg() && Op.getReg() == X86::RSP;
+    });
   }
 
   bool
   evaluateStackOffsetExpr(const MCInst &Inst, int64_t &Output,
                           std::pair<MCPhysReg, int64_t> Input1,
                           std::pair<MCPhysReg, int64_t> Input2) const override {
 
     auto getOperandVal = [&](MCPhysReg Reg) -> ErrorOr<int64_t> {
for (int I = 0, E = MCPlus::getNumPrimeOperands(Inst); I != E; ++I) {
       if (!HasFramePointer && Operand.isReg() && SPAliases[Operand.getReg()]) {
         DoesLeak = true;
         break;
       }
     }
 
     // If potential leak, check if it is not just writing to itself/sp/bp
     if (DoesLeak) {
-      for (int I = 0, E = NumDefs; I != E; ++I) {
-        const MCOperand &Operand = Inst.getOperand(I);
-        if (HasFramePointer && Operand.isReg() &&
-            SPBPAliases[Operand.getReg()]) {
-          DoesLeak = false;
-          break;
-        }
-        if (!HasFramePointer && Operand.isReg() &&
-            SPAliases[Operand.getReg()]) {
-          DoesLeak = false;
-          break;
-        }
-      }
+      DoesLeak = !any_of(defOperands(Inst), [&](const MCOperand &Operand) {
+        assert(Operand.isReg());
+        MCPhysReg Reg = Operand.getReg();
+        return HasFramePointer ? SPBPAliases[Reg] : SPAliases[Reg];
+      });
     }
     return DoesLeak;
   }
 
   bool addToImm(MCInst &Inst, int64_t &Amt, MCContext *Ctx) const override {
     unsigned ImmOpNo = -1U;
     int MemOpNo = getMemoryOperandNo(Inst);
     if (MemOpNo != -1)

 
   InstructionListType createInstrumentedIndirectCall(MCInst &&CallInst,
                                                      MCSymbol *HandlerFuncAddr,
                                                      int CallSiteID,
                                                      MCContext *Ctx) override {
     // Check if the target address expression used in the original indirect call
     // uses the stack pointer, which we are going to clobber.
     static BitVector SPAliases(getAliases(X86::RSP));
-    bool UsesSP = false;
-    // Skip defs.
-    for (unsigned I = Info->get(CallInst.getOpcode()).getNumDefs(),
-                  E = MCPlus::getNumPrimeOperands(CallInst);
-         I != E; ++I) {
-      const MCOperand &Operand = CallInst.getOperand(I);
-      if (Operand.isReg() && SPAliases[Operand.getReg()]) {
-        UsesSP = true;
-        break;
-      }
-    }
+    bool UsesSP = any_of(useOperands(CallInst), [&](const MCOperand &Op) {
+      return Op.isReg() && SPAliases[Op.getReg()];
+    });
 
     InstructionListType Insts;
     MCPhysReg TempReg = getIntArgRegister(0);
     // Code sequence used to enter indirect call instrumentation helper:
     //   push %rdi
     //   add $8, %rsp       ;; $rsp may be used in target, so fix it to prev val
     //   movq target, %rdi  ;; via convertIndirectCallTargetToLoad
     //   sub $8, %rsp       ;; restore correct stack value