[Attributor] Provide a generic interface to check live instructions

[jdoerfert]

Summary

Similar to Attributor::checkForAllCallSites, we now provide such
functionality for instructions of a certain opcode through
Attributor::checkForAllInstructions and the convenient wrapper
Attributor::checkForAllCallLikeInstructions. This cleans up code,
avoids duplication, and simplifies the usage of liveness information.

[sstefan1]

LGTM.

[plotfi]

This is build breaking on Linux and Darwin: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/15961/steps/build/logs/stdio

Reverting.

/Users/plotfi/Projects/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:2263:63: error: binding value of type 'const llvm::Function' to reference to type 'llvm::Function' drops 'const' qualifier
  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
                                                              ^
/Users/plotfi/Projects/llvm-project/llvm/include/llvm/Transforms/IPO/Attributor.h:336:58: note: passing argument to parameter 'F' here
  OpcodeInstMapTy &getOpcodeInstMapForFunction(Function &F) {
                                                         ^
1 error generated.

[plotfi]

nvm, beat me to it! Thanks! :-)

[jdoerfert]

In D65731#1616044, @plotfi wrote:

nvm, beat me to it! Thanks! :-)

I tested locally, saw the error (due to some rebasing), fixed it, tested, commited, and realized I didn't add the stuff somehow when I was flooded by emails... sorry for the noise.

llvm/trunk/include/llvm/Transforms/IPO/Attributor.h

void identifyDefaultAbstractAttributes(
       DenseSet<const char *> *Whitelist = nullptr);
 
   /// Check \p Pred on all function call sites.
   ///
   /// This method will evaluate \p Pred on call sites and return
   /// true if \p Pred holds in every call sites. However, this is only possible
   /// all call sites are known, hence the function has internal linkage.
   bool checkForAllCallSites(Function &F, std::function<bool(CallSite)> &Pred,
-                            bool RequireAllCallSites, AbstractAttribute &AA);
+                            AbstractAttribute &QueryingAA,
+                            bool RequireAllCallSites);
+
+  /// Check \p Pred on all instructions with an opcode present in \p Opcodes.
+  ///
+  /// This method will evaluate \p Pred on all instructions with an opcode
+  /// present in \p Opcode and return true if \p Pred holds on all of them.
+  bool checkForAllInstructions(
+      const Function &F, const llvm::function_ref<bool(Instruction &)> &Pred,
+      AbstractAttribute &QueryingAA, InformationCache &InfoCache,
+      const ArrayRef<unsigned> &Opcodes);
+
+  /// Check \p Pred on all call-like instructions (=CallBased derived).
+  ///
+  /// See checkForAllCallLikeInstructions(...) for more information.
+  bool checkForAllCallLikeInstructions(
+      const Function &F, const llvm::function_ref<bool(Instruction &)> &Pred,
+      AbstractAttribute &QueryingAA, InformationCache &InfoCache) {
+    return checkForAllInstructions(F, Pred, QueryingAA, InfoCache,
+                                   {(unsigned)Instruction::Invoke,
+                                    (unsigned)Instruction::CallBr,
+                                    (unsigned)Instruction::Call});
+  }
 
 private:
   /// The set of all abstract attributes.
   ///{
   using AAVector = SmallVector<AbstractAttribute *, 64>;
   AAVector AllAbstractAttributes;
   ///}
 

llvm/trunk/lib/Transforms/IPO/Attributor.cpp

struct AANoUnwindFunction final : public AANoUnwindImpl {
   AANoUnwindFunction(Function &F) : AANoUnwindImpl(F, IRP_FUNCTION) {}
 };
 
 ChangeStatus AANoUnwindImpl::updateImpl(Attributor &A,
                                         InformationCache &InfoCache) {
   Function &F = getAnchorScope();
 
   // The map from instruction opcodes to those instructions in the function.
-  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
   auto Opcodes = {
       (unsigned)Instruction::Invoke,      (unsigned)Instruction::CallBr,
       (unsigned)Instruction::Call,        (unsigned)Instruction::CleanupRet,
       (unsigned)Instruction::CatchSwitch, (unsigned)Instruction::Resume};
 
-  auto *LivenessAA = A.getAAFor<AAIsDead>(*this, F);
-
-  for (unsigned Opcode : Opcodes) {
-    for (Instruction *I : OpcodeInstMap[Opcode]) {
-      // Skip dead instructions.
-      if (LivenessAA && LivenessAA->isAssumedDead(I))
-        continue;
-
-      if (!I->mayThrow())
-        continue;
+  auto CheckForNoUnwind = [&](Instruction &I) {
+    if (!I.mayThrow())
+      return true;
 
-      auto *NoUnwindAA = A.getAAFor<AANoUnwind>(*this, *I);
+    auto *NoUnwindAA = A.getAAFor<AANoUnwind>(*this, I);
+    return NoUnwindAA && NoUnwindAA->isAssumedNoUnwind();
+  };
 
-      if (!NoUnwindAA || !NoUnwindAA->isAssumedNoUnwind())
-        return indicatePessimisticFixpoint();
-    }
-  }
+  if (!A.checkForAllInstructions(F, CheckForNoUnwind, *this, InfoCache,
+                                 Opcodes))
+
   return ChangeStatus::UNCHANGED;
 }
 
 /// --------------------- Function Return Values -------------------------------
 
 /// "Attribute" that collects all potential returned values and the return
 /// instructions that they arise from.
 ///
if (ICS)
       continue;
 
     if (!isVolatile(I) && !isNonRelaxedAtomic(I))
       continue;
 
     return indicatePessimisticFixpoint();
   }
 
-  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
-  auto Opcodes = {(unsigned)Instruction::Invoke, (unsigned)Instruction::CallBr,
-                  (unsigned)Instruction::Call};
-
-  for (unsigned Opcode : Opcodes) {
-    for (Instruction *I : OpcodeInstMap[Opcode]) {
-      // Skip assumed dead instructions.
-      if (LivenessAA && LivenessAA->isAssumedDead(I))
-        continue;
-      // At this point we handled all read/write effects and they are all
-      // nosync, so they can be skipped.
-      if (I->mayReadOrWriteMemory())
-        continue;
-
-      ImmutableCallSite ICS(I);
+  auto CheckForNoSync = [&](Instruction &I) {
+    if (I.mayReadOrWriteMemory())
+      return true;
 
-      // non-convergent and readnone imply nosync.
-      if (!ICS.isConvergent())
-        continue;
+    return !ImmutableCallSite(&I).isConvergent();
+  };
 
-      return indicatePessimisticFixpoint();
-    }
-  }
-
+  if (!A.checkForAllCallLikeInstructions(F, CheckForNoSync, *this, InfoCache))
   return ChangeStatus::UNCHANGED;
 }
 
 /// ------------------------ No-Free Attributes ----------------------------
 
 struct AANoFreeImpl : public AANoFree, BooleanState {
   IRPositionConstructorForward(AANoFreeImpl, AANoFree);
 

 struct AANoFreeFunction final : public AANoFreeImpl {
   AANoFreeFunction(Function &F) : AANoFreeImpl(F, IRP_FUNCTION) {}
 };
 
 ChangeStatus AANoFreeImpl::updateImpl(Attributor &A,
                                       InformationCache &InfoCache) {
   Function &F = getAnchorScope();
 
-  auto *LivenessAA = A.getAAFor<AAIsDead>(*this, F);
-
-  // The map from instruction opcodes to those instructions in the function.
-  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
+  auto CheckForNoFree = [&](Instruction &I) {
+    if (ImmutableCallSite(&I).hasFnAttr(Attribute::NoFree))
+      return true;
 
-  for (unsigned Opcode :
-       {(unsigned)Instruction::Invoke, (unsigned)Instruction::CallBr,
-        (unsigned)Instruction::Call}) {
-    for (Instruction *I : OpcodeInstMap[Opcode]) {
-      // Skip assumed dead instructions.
-      if (LivenessAA && LivenessAA->isAssumedDead(I))
-        continue;
-      auto ICS = ImmutableCallSite(I);
-      auto *NoFreeAA = A.getAAFor<AANoFreeImpl>(*this, *I);
+    auto *NoFreeAA = A.getAAFor<AANoFreeImpl>(*this, I);
+    return NoFreeAA && NoFreeAA->isAssumedNoFree();
+  };
 
-      if ((!NoFreeAA || !NoFreeAA->isAssumedNoFree()) &&
-          !ICS.hasFnAttr(Attribute::NoFree))
-        return indicatePessimisticFixpoint();
-    }
-  }
+  if (!A.checkForAllCallLikeInstructions(F, CheckForNoFree, *this, InfoCache))
   return ChangeStatus::UNCHANGED;
 }
 
 /// ------------------------ NonNull Argument Attribute ------------------------
 struct AANonNullImpl : AANonNull, BooleanState {
   IRPositionConstructorForward(AANonNullImpl, AANonNull);
 
   /// See AbstractAttribute::getState()
if (CS.paramHasAttr(ArgNo, Attribute::NonNull))
       return true;
 
     Value *V = CS.getArgOperand(ArgNo);
     if (isKnownNonZero(V, getAnchorScope().getParent()->getDataLayout()))
       return true;
 
     return false;
   };
-  if (!A.checkForAllCallSites(F, CallSiteCheck, true, *this))
+  if (!A.checkForAllCallSites(F, CallSiteCheck, *this, true))
     return indicatePessimisticFixpoint();
   return ChangeStatus::UNCHANGED;
 }
 
 ChangeStatus
 AANonNullCallSiteArgument::updateImpl(Attributor &A,
                                       InformationCache &InfoCache) {
   // NOTE: Never look at the argument of the callee in this method.
void AAWillReturnFunction::initialize(Attributor &A,
   Function &F = getAnchorScope();
 
   if (containsPossiblyEndlessLoop(F))
     indicatePessimisticFixpoint();
 }
 
 ChangeStatus AAWillReturnFunction::updateImpl(Attributor &A,
                                               InformationCache &InfoCache) {
-  Function &F = getAnchorScope();
-
+  const Function &F = getAnchorScope();
   // The map from instruction opcodes to those instructions in the function.
-  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
-
-  auto *LivenessAA = A.getAAFor<AAIsDead>(*this, F);
-
-  for (unsigned Opcode :
-       {(unsigned)Instruction::Invoke, (unsigned)Instruction::CallBr,
-        (unsigned)Instruction::Call}) {
-    for (Instruction *I : OpcodeInstMap[Opcode]) {
-      // Skip assumed dead instructions.
-      if (LivenessAA && LivenessAA->isAssumedDead(I))
-        continue;
-
-      auto ICS = ImmutableCallSite(I);
 
-      if (ICS.hasFnAttr(Attribute::WillReturn))
-        continue;
+  auto CheckForWillReturn = [&](Instruction &I) {
+    ImmutableCallSite ICS(&I);
+      return true;
 
-      auto *WillReturnAA = A.getAAFor<AAWillReturn>(*this, *I);
-      if (!WillReturnAA || !WillReturnAA->isAssumedWillReturn())
-        return indicatePessimisticFixpoint();
+    auto *WillReturnAA = A.getAAFor<AAWillReturn>(*this, I);
+      return false;
 
-      auto *NoRecurseAA = A.getAAFor<AANoRecurse>(*this, *I);
+    // FIXME: Prohibit any recursion for now.
+    if (ICS.hasFnAttr(Attribute::NoRecurse))
+      return true;
+
+    auto *NoRecurseAA = A.getAAFor<AANoRecurse>(*this, I);
+    return NoRecurseAA && NoRecurseAA->isAssumedNoRecurse();
+  };
 
-      // FIXME: (i) Prohibit any recursion for now.
-      //        (ii) AANoRecurse isn't implemented yet so currently any call is
-      //        regarded as having recursion.
-      //       Code below should be
-      //       if ((!NoRecurseAA || !NoRecurseAA->isAssumedNoRecurse()) &&
-      if (!NoRecurseAA && !ICS.hasFnAttr(Attribute::NoRecurse))
-        return indicatePessimisticFixpoint();
-    }
-  }
+  if (!A.checkForAllCallLikeInstructions(F, CheckForWillReturn, *this,
+                                         InfoCache))
 
   return ChangeStatus::UNCHANGED;
 }
 
 /// ------------------------ NoAlias Argument Attribute ------------------------
 
 struct AANoAliasImpl : AANoAlias, BooleanState {
   IRPositionConstructorForward(AANoAliasImpl, AANoAlias);
std::function<bool(CallSite)> CallSiteCheck = [&](CallSite CS) -> bool {
     }
 
     takeAssumedDerefBytesMinimum(computeAssumedDerefenceableBytes(
         A, *CS.getArgOperand(ArgNo), IsNonNull, IsGlobal));
 
     return isValidState();
   };
 
-  if (!A.checkForAllCallSites(F, CallSiteCheck, true, *this))
+  if (!A.checkForAllCallSites(F, CallSiteCheck, *this, true))
     return indicatePessimisticFixpoint();
 
   updateAssumedNonNullGlobalState(IsNonNull, IsGlobal);
 
   return BeforeState == static_cast<DerefState>(*this) ? ChangeStatus::UNCHANGED
                                                        : ChangeStatus::CHANGED;
 }
 
if (AlignAA) {
       }
     }
 
     Value *V = CS.getArgOperand(ArgNo);
     takeAssumedMinimum(V->getPointerAlignment(DL));
     return isValidState();
   };
 
-  if (!A.checkForAllCallSites(F, CallSiteCheck, true, *this))
+  if (!A.checkForAllCallSites(F, CallSiteCheck, *this, true))
     indicatePessimisticFixpoint();
 
   return BeforeState == getAssumed() ? ChangeStatus::UNCHANGED
                                      : ChangeStatus ::CHANGED;
 }
 
 struct AAAlignCallSiteArgument final : AAAlignImpl {
 
void initialize(Attributor &A, InformationCache &InfoCache) override {
     Function &F = getAnchorScope();
     if (F.hasFnAttribute(getAttrKind()))
       indicateOptimisticFixpoint();
   }
 
   /// See AbstractAttribute::updateImpl(Attributor &A).
   virtual ChangeStatus updateImpl(Attributor &A,
                                   InformationCache &InfoCache) override {
-    Function &F = getAnchorScope();
-
-    // The map from instruction opcodes to those instructions in the function.
-    auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
-
-    // Look at all return instructions.
-    auto &ReturnInsts = OpcodeInstMap[Instruction::Ret];
-    if (ReturnInsts.empty())
-      return indicateOptimisticFixpoint();
-
-    auto *LivenessAA = A.getAAFor<AAIsDead>(*this, F);
-    if (!LivenessAA ||
-        LivenessAA->isLiveInstSet(ReturnInsts.begin(), ReturnInsts.end()))
+    const Function &F = getAnchorScope();
+    auto CheckForNoReturn = [](Instruction &) { return false; };
+    if (!A.checkForAllInstructions(F, CheckForNoReturn, *this, InfoCache,
+                                   {(unsigned)Instruction::Ret}))
       return indicatePessimisticFixpoint();
-
     return ChangeStatus::UNCHANGED;
   }
 };
 
 struct AANoReturnFunction final : AANoReturnImpl {
   AANoReturnFunction(Function &F) : AANoReturnImpl(F, IRP_FUNCTION) {}
 };
 
 /// ----------------------------------------------------------------------------
 ///                               Attributor
 /// ----------------------------------------------------------------------------
 
 bool Attributor::checkForAllCallSites(Function &F,
                                       std::function<bool(CallSite)> &Pred,
-                                      bool RequireAllCallSites,
-                                      AbstractAttribute &AA) {
+                                      AbstractAttribute &QueryingAA,
+                                      bool RequireAllCallSites) {
   // We can try to determine information from
   // the call sites. However, this is only possible all call sites are known,
   // hence the function has internal linkage.
   if (RequireAllCallSites && !F.hasInternalLinkage()) {
     LLVM_DEBUG(
         dbgs()
         << "Attributor: Function " << F.getName()
         << " has no internal linkage, hence not all call sites are known\n");
     return false;
   }
 
   for (const Use &U : F.uses()) {
     Instruction *I = cast<Instruction>(U.getUser());
     Function *AnchorValue = I->getParent()->getParent();
 
-    auto *LivenessAA = getAAFor<AAIsDead>(AA, *AnchorValue);
+    auto *LivenessAA = getAAFor<AAIsDead>(QueryingAA, *AnchorValue);
 
     // Skip dead calls.
     if (LivenessAA && LivenessAA->isAssumedDead(I))
       continue;
 
     CallSite CS(U.getUser());
     if (!CS || !CS.isCallee(&U) || !CS.getCaller()->hasExactDefinition()) {
       if (!RequireAllCallSites)
for (const Use &U : F.uses()) {
     LLVM_DEBUG(dbgs() << "Attributor: Call site callback failed for "
                       << *CS.getInstruction() << "\n");
     return false;
   }
 
   return true;
 }
 
+bool Attributor::checkForAllInstructions(
+    const Function &F, const llvm::function_ref<bool(Instruction &)> &Pred,
+    AbstractAttribute &QueryingAA, InformationCache &InfoCache,
+    const ArrayRef<unsigned> &Opcodes) {
+
+  auto *LivenessAA = getAAFor<AAIsDead>(QueryingAA, F);
+
+  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
+  for (unsigned Opcode : Opcodes) {
+    for (Instruction *I : OpcodeInstMap[Opcode]) {
+      // Skip dead instructions.
+      if (LivenessAA && LivenessAA->isAssumedDead(I))
+        continue;
+
+      if (!Pred(*I))
+        return false;
+    }
+  }
+
+  return true;
+}
+
 ChangeStatus Attributor::run(InformationCache &InfoCache) {
   // Initialize all abstract attributes.
   for (AbstractAttribute *AA : AllAbstractAttributes)
     AA->initialize(*this, InfoCache);
 
   LLVM_DEBUG(dbgs() << "[Attributor] Identified and initialized "
                     << AllAbstractAttributes.size()
                     << " abstract attributes.\n");