[Attributor] Introduce checkForAllReadWriteInstructions(...).

[sstefan1]

Summary

Similarly to D65731 Attributor::checkForAllReadWriteInstructions is introduced.

[jdoerfert]

One small request, otherwise LGTM.

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

300 ↗

(On Diff #213700)

bool(Instruction &)

llvm/lib/Transforms/IPO/Attributor.cpp

944 ↗

(On Diff #213700)

It's much nice now that the concrete AAs don't need to deal with liveness and the infocache etc. but still get the benefits.

[sstefan1]

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

300 ↗

(On Diff #213700)

This will require changes in AANoSync helper functions. I'll change those as well.

llvm/lib/Transforms/IPO/Attributor.cpp

944 ↗

(On Diff #213700)

Agreed!

[sstefan1]

• address comment.

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

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});
   }
 
+  /// Check \p Pred on all Read/Write instructions.
+  ///
+  /// This method will evaluate \p Pred on all instructions that read or write
+  /// to memory present in \p InfoCache and return true if \p Pred holds on all
+  /// of them.
+  bool checkForAllReadWriteInstructions(
+      const Function &F, const llvm::function_ref<bool(Instruction &)> &Pred,
+      AbstractAttribute &QueryingAA, InformationCache &InfoCache);
+
 private:
   /// The set of all abstract attributes.
   ///{
   using AAVector = SmallVector<AbstractAttribute *, 64>;
   AAVector AllAbstractAttributes;
   ///}
 
   /// A nested map to lookup abstract attributes based on the anchored value and

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

default:
     return false;
   }
 }
 
 ChangeStatus AANoSyncImpl::updateImpl(Attributor &A,
                                       InformationCache &InfoCache) {
   Function &F = getAnchorScope();
 
-  auto *LivenessAA = A.getAAFor<AAIsDead>(*this, F);
-
+  auto CheckRWInstForNoSync = [&](Instruction &I) {
     /// We are looking for volatile instructions or Non-Relaxed atomics.
     /// FIXME: We should ipmrove the handling of intrinsics.
-  for (Instruction *I : InfoCache.getReadOrWriteInstsForFunction(F)) {
-    // Skip assumed dead instructions.
-    if (LivenessAA && LivenessAA->isAssumedDead(I))
-      continue;
 
-    ImmutableCallSite ICS(I);
-    auto *NoSyncAA = A.getAAFor<AANoSyncImpl>(*this, *I);
+    ImmutableCallSite ICS(&I);
+    auto *NoSyncAA = A.getAAFor<AANoSyncImpl>(*this, I);
 
-    if (isa<IntrinsicInst>(I) && isNoSyncIntrinsic(I))
-      continue;
+    if (isa<IntrinsicInst>(&I) && isNoSyncIntrinsic(&I))
+      return true;
 
     if (ICS && (!NoSyncAA || !NoSyncAA->isAssumedNoSync()) &&
         !ICS.hasFnAttr(Attribute::NoSync))
-      return indicatePessimisticFixpoint();
+      return false;
 
     if (ICS)
-      continue;
+      return true;
 
-    if (!isVolatile(I) && !isNonRelaxedAtomic(I))
-      continue;
+    if (!isVolatile(&I) && !isNonRelaxedAtomic(&I))
+      return true;
 
-    return indicatePessimisticFixpoint();
-  }
+    return false;
+  };
 
   auto CheckForNoSync = [&](Instruction &I) {
     // At this point we handled all read/write effects and they are all
     // nosync, so they can be skipped.
     if (I.mayReadOrWriteMemory())
       return true;
 
     // non-convergent and readnone imply nosync.
     return !ImmutableCallSite(&I).isConvergent();
   };
 
-  if (!A.checkForAllCallLikeInstructions(F, CheckForNoSync, *this, InfoCache))
+  if (!A.checkForAllReadWriteInstructions(F, CheckRWInstForNoSync, *this,
+                                          InfoCache) ||
+      !A.checkForAllCallLikeInstructions(F, CheckForNoSync, *this, InfoCache))
     return indicatePessimisticFixpoint();
+
   return ChangeStatus::UNCHANGED;
 }
 
 /// ------------------------ No-Free Attributes ----------------------------
 
 struct AANoFreeImpl : public AANoFree, BooleanState {
   IRPositionConstructorForward(AANoFreeImpl, AANoFree);
 
for (Instruction *I : OpcodeInstMap[Opcode]) {
       if (!Pred(*I))
         return false;
     }
   }
 
   return true;
 }
 
+bool Attributor::checkForAllReadWriteInstructions(
+    const Function &F, const llvm::function_ref<bool(Instruction &)> &Pred,
+    AbstractAttribute &QueryingAA, InformationCache &InfoCache) {
+
+  auto *LivenessAA = getAAFor<AAIsDead>(QueryingAA, F);
+
+  for (Instruction *I : InfoCache.getReadOrWriteInstsForFunction(F)) {
+    // 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");