diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h --- a/llvm/include/llvm/Transforms/IPO/Attributor.h +++ b/llvm/include/llvm/Transforms/IPO/Attributor.h @@ -591,9 +591,16 @@ /// most optimistic information for other abstract attributes in-flight, e.g. /// the one reasoning about the "captured" state for the argument or the one /// reasoning on the memory access behavior of the function as a whole. + /// + /// If the flag \p TrackDependence is set to false the dependence from + /// \p QueryingAA to the return abstract attribute is not automatically + /// recorded. This should only be used if the caller will record the + /// dependence explicitly if necessary, thus if it the returned abstract + /// attribute is used for reasoning. To record the dependences explicitly use + /// the `Attributor::recordDependence` method. template const AAType &getAAFor(const AbstractAttribute &QueryingAA, - const IRPosition &IRP) { + const IRPosition &IRP, bool TrackDependence = true) { static_assert(std::is_base_of::value, "Cannot query an attribute with a type not derived from " "'AbstractAttribute'!"); @@ -605,7 +612,7 @@ if (AAType *AA = static_cast( KindToAbstractAttributeMap.lookup(&AAType::ID))) { // Do not registr a dependence on an attribute with an invalid state. - if (AA->getState().isValidState()) + if (TrackDependence && AA->getState().isValidState()) QueryMap[AA].insert(const_cast(&QueryingAA)); return *AA; } @@ -613,11 +620,24 @@ // No matching attribute found, create one. auto &AA = AAType::createForPosition(IRP, *this); registerAA(AA); - if (AA.getState().isValidState()) + if (TrackDependence && AA.getState().isValidState()) QueryMap[&AA].insert(const_cast(&QueryingAA)); return AA; } + /// Explicitly record a dependence from \p FromAA to \p ToAA, that is if + /// \p FromAA changes \p ToAA should be updated as well. + /// + /// This method should be used in conjunction with the `getAAFor` method and + /// with the TrackDependence flag passed to the method set to false. This can + /// be beneficial to avoid false dependences but it requires the users of + /// `getAAFor` to explicitly record true dependences through this method. + void recordDependence(const AbstractAttribute &FromAA, + const AbstractAttribute &ToAA) { + QueryMap[const_cast(&FromAA)].insert( + const_cast(&ToAA)); + } + /// Introduce a new abstract attribute into the fixpoint analysis. /// /// Note that ownership of the attribute is given to the Attributor. It will diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -146,7 +146,9 @@ const AAIsDead *LivenessAA = nullptr; if (IRP.getAnchorScope()) LivenessAA = &A.getAAFor( - QueryingAA, IRPosition::function(*IRP.getAnchorScope())); + QueryingAA, IRPosition::function(*IRP.getAnchorScope()), + /* TrackDependence */ false); + bool AnyDead = false; // TODO: Use Positions here to allow context sensitivity in VisitValueCB SmallPtrSet Visited; @@ -199,8 +201,11 @@ "Expected liveness in the presence of instructions!"); for (unsigned u = 0, e = PHI->getNumIncomingValues(); u < e; u++) { const BasicBlock *IncomingBB = PHI->getIncomingBlock(u); - if (!LivenessAA->isAssumedDead(IncomingBB->getTerminator())) - Worklist.push_back(PHI->getIncomingValue(u)); + if (LivenessAA->isAssumedDead(IncomingBB->getTerminator())) { + AnyDead =true; + continue; + } + Worklist.push_back(PHI->getIncomingValue(u)); } continue; } @@ -210,6 +215,10 @@ return false; } while (!Worklist.empty()); + // If we actually used liveness information so we have to record a dependence. + if (AnyDead) + A.recordDependence(*LivenessAA, QueryingAA); + // All values have been visited. return true; } @@ -2282,7 +2291,8 @@ if (!LivenessAA) LivenessAA = - &getAAFor(AA, IRPosition::function(*CtxI->getFunction())); + &getAAFor(AA, IRPosition::function(*CtxI->getFunction()), + /* TrackDependence */ false); // Don't check liveness for AAIsDead. if (&AA == LivenessAA) @@ -2291,8 +2301,9 @@ if (!LivenessAA->isAssumedDead(CtxI)) return false; - // TODO: Do not track dependences automatically but add it here as only a - // "is-assumed-dead" result causes a dependence. + // We actually used liveness information so we have to record a dependence. + recordDependence(*LivenessAA, AA); + return true; } @@ -2323,12 +2334,16 @@ Function *Caller = I->getFunction(); - const auto &LivenessAA = - getAAFor(QueryingAA, IRPosition::function(*Caller)); + const auto &LivenessAA = getAAFor( + QueryingAA, IRPosition::function(*Caller), /* TrackDependence */ false); // Skip dead calls. - if (LivenessAA.isAssumedDead(I)) + if (LivenessAA.isAssumedDead(I)) { + // We actually used liveness information so we have to record a + // dependence. + recordDependence(LivenessAA, QueryingAA); continue; + } CallSite CS(U.getUser()); if (!CS || !CS.isCallee(&U) || !CS.getCaller()->hasExactDefinition()) { @@ -2405,21 +2420,29 @@ return false; const IRPosition &QueryIRP = IRPosition::function_scope(IRP); - const auto &LivenessAA = getAAFor(QueryingAA, QueryIRP); + const auto &LivenessAA = + getAAFor(QueryingAA, QueryIRP, /* TrackDependence */ false); + bool AnyDead = false; auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(*AssociatedFunction); for (unsigned Opcode : Opcodes) { for (Instruction *I : OpcodeInstMap[Opcode]) { // Skip dead instructions. - if (LivenessAA.isAssumedDead(I)) + if (LivenessAA.isAssumedDead(I)) { + AnyDead = true; continue; + } if (!Pred(*I)) return false; } } + // If we actually used liveness information so we have to record a dependence. + if (AnyDead) + recordDependence(LivenessAA, QueryingAA); + return true; } @@ -2432,19 +2455,26 @@ if (!AssociatedFunction) return false; - const auto &LivenessAA = - getAAFor(QueryingAA, QueryingAA.getIRPosition()); + const auto &LivenessAA = getAAFor( + QueryingAA, QueryingAA.getIRPosition(), /* TrackDependence */ false); + bool AnyDead = false; for (Instruction *I : InfoCache.getReadOrWriteInstsForFunction(*AssociatedFunction)) { // Skip dead instructions. - if (LivenessAA.isAssumedDead(I)) + if (LivenessAA.isAssumedDead(I)) { + AnyDead = true; continue; + } if (!Pred(*I)) return false; } + // If we actually used liveness information so we have to record a dependence. + if (AnyDead) + recordDependence(LivenessAA, QueryingAA); + return true; } diff --git a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll --- a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll +++ b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll @@ -1,5 +1,5 @@ ; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR -; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations=26 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR +; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations=27 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR ; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH ; ; Test cases specifically designed for the "returned" argument attribute.