[llvm] 5eba784 - [Attributor] Use checkForAllUses instead of custom use tracking
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 19 23:41:10 PDT 2021
Author: Johannes Doerfert
Date: 2021-07-20T01:39:33-05:00
New Revision: 5eba7846a5cb3777bf1178da5bbd86f117157d98
URL: https://github.com/llvm/llvm-project/commit/5eba7846a5cb3777bf1178da5bbd86f117157d98
DIFF: https://github.com/llvm/llvm-project/commit/5eba7846a5cb3777bf1178da5bbd86f117157d98.diff
LOG: [Attributor] Use checkForAllUses instead of custom use tracking
AAMemoryBehaviorFloating used a custom use tracking mechanism even
though checkForAllUses exists and is already more powerful. Further,
AAMemoryBehaviorFloating uses AANoCapture to guarantee that there are no
aliases and following the uses is sufficient. This is an OK assumption
if checkForAllUses is used but custom tracking is easily out of sync
with AANoCapture and problems follow.
Added:
Modified:
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/test/Transforms/Attributor/depgraph.ll
llvm/test/Transforms/Attributor/heap_to_stack.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 97537559629c0..229f09c5d5dae 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -6813,12 +6813,6 @@ struct AAMemoryBehaviorFloating : AAMemoryBehaviorImpl {
AAMemoryBehaviorFloating(const IRPosition &IRP, Attributor &A)
: AAMemoryBehaviorImpl(IRP, A) {}
- /// See AbstractAttribute::initialize(...).
- void initialize(Attributor &A) override {
- AAMemoryBehaviorImpl::initialize(A);
- addUsesOf(A, getAssociatedValue());
- }
-
/// See AbstractAttribute::updateImpl(...).
ChangeStatus updateImpl(Attributor &A) override;
@@ -6835,21 +6829,11 @@ struct AAMemoryBehaviorFloating : AAMemoryBehaviorImpl {
private:
/// Return true if users of \p UserI might access the underlying
/// variable/location described by \p U and should therefore be analyzed.
- bool followUsersOfUseIn(Attributor &A, const Use *U,
+ bool followUsersOfUseIn(Attributor &A, const Use &U,
const Instruction *UserI);
/// Update the state according to the effect of use \p U in \p UserI.
- void analyzeUseIn(Attributor &A, const Use *U, const Instruction *UserI);
-
-protected:
- /// Add the uses of \p V to the `Uses` set we look at during the update step.
- void addUsesOf(Attributor &A, const Value &V);
-
- /// Container for (transitive) uses of the associated argument.
- SmallVector<const Use *, 8> Uses;
-
- /// Set to remember the uses we already traversed.
- SmallPtrSet<const Use *, 8> Visited;
+ void analyzeUseIn(Attributor &A, const Use &U, const Instruction *UserI);
};
/// Memory behavior attribute for function argument.
@@ -6871,11 +6855,8 @@ struct AAMemoryBehaviorArgument : AAMemoryBehaviorFloating {
// Initialize the use vector with all direct uses of the associated value.
Argument *Arg = getAssociatedArgument();
- if (!Arg || !A.isFunctionIPOAmendable(*(Arg->getParent()))) {
+ if (!Arg || !A.isFunctionIPOAmendable(*(Arg->getParent())))
indicatePessimisticFixpoint();
- } else {
- addUsesOf(A, *Arg);
- }
}
ChangeStatus manifest(Attributor &A) override {
@@ -7108,65 +7089,34 @@ ChangeStatus AAMemoryBehaviorFloating::updateImpl(Attributor &A) {
// The current assumed state used to determine a change.
auto AssumedState = S.getAssumed();
- // Liveness information to exclude dead users.
- // TODO: Take the FnPos once we have call site specific liveness information.
- const auto &LivenessAA = A.getAAFor<AAIsDead>(
- *this, IRPosition::function(*IRP.getAssociatedFunction()),
- DepClassTy::NONE);
-
// Visit and expand uses until all are analyzed or a fixpoint is reached.
- for (unsigned i = 0; i < Uses.size() && !isAtFixpoint(); i++) {
- const Use *U = Uses[i];
- Instruction *UserI = cast<Instruction>(U->getUser());
- bool UsedAssumedInformation = false;
- LLVM_DEBUG(dbgs() << "[AAMemoryBehavior] Use: " << **U << " in " << *UserI
- << " [Dead: "
- << (A.isAssumedDead(*U, this, &LivenessAA,
- UsedAssumedInformation))
- << "]\n");
- if (A.isAssumedDead(*U, this, &LivenessAA, UsedAssumedInformation))
- continue;
+ auto UsePred = [&](const Use &U, bool &Follow) -> bool {
+ Instruction *UserI = cast<Instruction>(U.getUser());
+ LLVM_DEBUG(dbgs() << "[AAMemoryBehavior] Use: " << *U << " in " << *UserI
+ << " \n");
// Droppable users, e.g., llvm::assume does not actually perform any action.
if (UserI->isDroppable())
- continue;
+ return true;
// Check if the users of UserI should also be visited.
- if (followUsersOfUseIn(A, U, UserI))
- addUsesOf(A, *UserI);
+ Follow = followUsersOfUseIn(A, U, UserI);
// If UserI might touch memory we analyze the use in detail.
if (UserI->mayReadOrWriteMemory())
analyzeUseIn(A, U, UserI);
- }
- return (AssumedState != getAssumed()) ? ChangeStatus::CHANGED
- : ChangeStatus::UNCHANGED;
-}
-
-void AAMemoryBehaviorFloating::addUsesOf(Attributor &A, const Value &V) {
- SmallVector<const Use *, 8> WL;
- for (const Use &U : V.uses())
- WL.push_back(&U);
+ return !isAtFixpoint();
+ };
- while (!WL.empty()) {
- const Use *U = WL.pop_back_val();
- if (!Visited.insert(U).second)
- continue;
+ if (!A.checkForAllUses(UsePred, *this, getAssociatedValue()))
+ return indicatePessimisticFixpoint();
- const Instruction *UserI = cast<Instruction>(U->getUser());
- if (UserI->mayReadOrWriteMemory()) {
- Uses.push_back(U);
- continue;
- }
- if (!followUsersOfUseIn(A, U, UserI))
- continue;
- for (const Use &UU : UserI->uses())
- WL.push_back(&UU);
- }
+ return (AssumedState != getAssumed()) ? ChangeStatus::CHANGED
+ : ChangeStatus::UNCHANGED;
}
-bool AAMemoryBehaviorFloating::followUsersOfUseIn(Attributor &A, const Use *U,
+bool AAMemoryBehaviorFloating::followUsersOfUseIn(Attributor &A, const Use &U,
const Instruction *UserI) {
// The loaded value is unrelated to the pointer argument, no need to
// follow the users of the load.
@@ -7176,7 +7126,7 @@ bool AAMemoryBehaviorFloating::followUsersOfUseIn(Attributor &A, const Use *U,
// By default we follow all uses assuming UserI might leak information on U,
// we have special handling for call sites operands though.
const auto *CB = dyn_cast<CallBase>(UserI);
- if (!CB || !CB->isArgOperand(U))
+ if (!CB || !CB->isArgOperand(&U))
return true;
// If the use is a call argument known not to be captured, the users of
@@ -7185,8 +7135,8 @@ bool AAMemoryBehaviorFloating::followUsersOfUseIn(Attributor &A, const Use *U,
// general capturing of the underlying argument. The reason is that the
// call might the argument "through return", which we allow and for which we
// need to check call users.
- if (U->get()->getType()->isPointerTy()) {
- unsigned ArgNo = CB->getArgOperandNo(U);
+ if (U.get()->getType()->isPointerTy()) {
+ unsigned ArgNo = CB->getArgOperandNo(&U);
const auto &ArgNoCaptureAA = A.getAAFor<AANoCapture>(
*this, IRPosition::callsite_argument(*CB, ArgNo), DepClassTy::OPTIONAL);
return !ArgNoCaptureAA.isAssumedNoCapture();
@@ -7195,7 +7145,7 @@ bool AAMemoryBehaviorFloating::followUsersOfUseIn(Attributor &A, const Use *U,
return true;
}
-void AAMemoryBehaviorFloating::analyzeUseIn(Attributor &A, const Use *U,
+void AAMemoryBehaviorFloating::analyzeUseIn(Attributor &A, const Use &U,
const Instruction *UserI) {
assert(UserI->mayReadOrWriteMemory());
@@ -7212,7 +7162,7 @@ void AAMemoryBehaviorFloating::analyzeUseIn(Attributor &A, const Use *U,
// Stores cause the NO_WRITES property to disappear if the use is the
// pointer operand. Note that we do assume that capturing was taken care of
// somewhere else.
- if (cast<StoreInst>(UserI)->getPointerOperand() == U->get())
+ if (cast<StoreInst>(UserI)->getPointerOperand() == U.get())
removeAssumedBits(NO_WRITES);
return;
@@ -7224,14 +7174,14 @@ void AAMemoryBehaviorFloating::analyzeUseIn(Attributor &A, const Use *U,
const auto *CB = cast<CallBase>(UserI);
// Give up on operand bundles.
- if (CB->isBundleOperand(U)) {
+ if (CB->isBundleOperand(&U)) {
indicatePessimisticFixpoint();
return;
}
// Calling a function does read the function pointer, maybe write it if the
// function is self-modifying.
- if (CB->isCallee(U)) {
+ if (CB->isCallee(&U)) {
removeAssumedBits(NO_READS);
break;
}
@@ -7239,8 +7189,8 @@ void AAMemoryBehaviorFloating::analyzeUseIn(Attributor &A, const Use *U,
// Adjust the possible access behavior based on the information on the
// argument.
IRPosition Pos;
- if (U->get()->getType()->isPointerTy())
- Pos = IRPosition::callsite_argument(*CB, CB->getArgOperandNo(U));
+ if (U.get()->getType()->isPointerTy())
+ Pos = IRPosition::callsite_argument(*CB, CB->getArgOperandNo(&U));
else
Pos = IRPosition::callsite_function(*CB);
const auto &MemBehaviorAA =
diff --git a/llvm/test/Transforms/Attributor/depgraph.ll b/llvm/test/Transforms/Attributor/depgraph.ll
index b6e1654a59db6..06283ea376578 100644
--- a/llvm/test/Transforms/Attributor/depgraph.ll
+++ b/llvm/test/Transforms/Attributor/depgraph.ll
@@ -230,12 +230,12 @@ define i32* @checkAndAdvance(i32* align 16 %0) {
; GRAPH-EMPTY:
; GRAPH-NEXT: [AAIsDead] for CtxI ' ret i32* %.0' at position {flt: [@-1]} with state assumed-live
; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAIsDead] for CtxI ' br label %8' at position {flt: [@-1]} with state assumed-live
-; GRAPH-EMPTY:
; GRAPH-NEXT: [AAIsDead] for CtxI ' %5 = getelementptr inbounds i32, i32* %0, i64 4' at position {flt: [@-1]} with state assumed-live
; GRAPH-EMPTY:
; GRAPH-NEXT: [AAIsDead] for CtxI ' br label %8' at position {flt: [@-1]} with state assumed-live
; GRAPH-EMPTY:
+; GRAPH-NEXT: [AAIsDead] for CtxI ' br label %8' at position {flt: [@-1]} with state assumed-live
+; GRAPH-EMPTY:
; GRAPH-NEXT: [AANoAlias] for CtxI ' %5 = getelementptr inbounds i32, i32* %0, i64 4' at position {flt: [@-1]} with state may-alias
; GRAPH-EMPTY:
; GRAPH-NEXT: [AAMemoryLocation] for CtxI ' %6 = call i32* @checkAndAdvance(i32* %5)' at position {cs: [@-1]} with state memory:argument
diff --git a/llvm/test/Transforms/Attributor/heap_to_stack.ll b/llvm/test/Transforms/Attributor/heap_to_stack.ll
index 7f1b6d7d707d4..9979e0e1d9ca0 100644
--- a/llvm/test/Transforms/Attributor/heap_to_stack.ll
+++ b/llvm/test/Transforms/Attributor/heap_to_stack.ll
@@ -32,7 +32,7 @@ declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) nounwind
define void @h2s_value_simplify_interaction(i1 %c, i8* %A) {
; IS________OPM-LABEL: define {{[^@]+}}@h2s_value_simplify_interaction
-; IS________OPM-SAME: (i1 [[C:%.*]], i8* nocapture nofree [[A:%.*]]) {
+; IS________OPM-SAME: (i1 [[C:%.*]], i8* nocapture nofree readnone [[A:%.*]]) {
; IS________OPM-NEXT: entry:
; IS________OPM-NEXT: [[M:%.*]] = tail call noalias i8* @malloc(i64 noundef 4)
; IS________OPM-NEXT: br i1 [[C]], label [[T:%.*]], label [[F:%.*]]
@@ -55,7 +55,7 @@ define void @h2s_value_simplify_interaction(i1 %c, i8* %A) {
; IS________OPM-NEXT: ret void
;
; IS________NPM-LABEL: define {{[^@]+}}@h2s_value_simplify_interaction
-; IS________NPM-SAME: (i1 [[C:%.*]], i8* nocapture nofree [[A:%.*]]) {
+; IS________NPM-SAME: (i1 [[C:%.*]], i8* nocapture nofree readnone [[A:%.*]]) {
; IS________NPM-NEXT: entry:
; IS________NPM-NEXT: [[TMP0:%.*]] = alloca i8, i64 4, align 1
; IS________NPM-NEXT: br i1 [[C]], label [[T:%.*]], label [[F:%.*]]
More information about the llvm-commits
mailing list