[llvm] af48351 - [Attributor][FIX] Stabilize the state of AAReturnedValues each update
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Tue May 12 19:02:05 PDT 2020
Author: Johannes Doerfert
Date: 2020-05-12T21:00:30-05:00
New Revision: af48351cc8fc63148e8cfd10a936b2d5d0c2f078
URL: https://github.com/llvm/llvm-project/commit/af48351cc8fc63148e8cfd10a936b2d5d0c2f078
DIFF: https://github.com/llvm/llvm-project/commit/af48351cc8fc63148e8cfd10a936b2d5d0c2f078.diff
LOG: [Attributor][FIX] Stabilize the state of AAReturnedValues each update
For AAReturnedValues we treated new and existing information differently
in the updateImpl. Only the latter was properly analyzed and
categorized. The former was thought to be analyzed in the subsequent
update. Since the Attributor does not support "self-updates" we need to
make sure the state is "stable" after each updateImpl invocation. That
is, if the surrounding information does not change, the state is valid.
Now we make sure all return values have been handled and properly
categorized each iteration. We might not update again if we have not
requested a non-fix attribute so we cannot "wait" for the next update to
analyze a new return value.
Bug reported by @sdmitriev.
Added:
Modified:
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/test/Transforms/Attributor/returned.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index c764e9f2acc8..aa459c4b050a 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -1009,20 +1009,23 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
return indicatePessimisticFixpoint();
// Once returned values "directly" present in the code are handled we try to
- // resolve returned calls.
+ // resolve returned calls. To avoid modifications to the ReturnedValues map
+ // while we iterate over it we kept record of potential new entries in a copy
+ // map, NewRVsMap.
decltype(ReturnedValues) NewRVsMap;
- for (auto &It : ReturnedValues) {
- LLVM_DEBUG(dbgs() << "[AAReturnedValues] Returned value: " << *It.first
- << " by #" << It.second.size() << " RIs\n");
- CallBase *CB = dyn_cast<CallBase>(It.first);
+
+ auto HandleReturnValue = [&](Value *RV, SmallSetVector<ReturnInst *, 4> &RIs) {
+ LLVM_DEBUG(dbgs() << "[AAReturnedValues] Returned value: " << *RV
+ << " by #" << RIs.size() << " RIs\n");
+ CallBase *CB = dyn_cast<CallBase>(RV);
if (!CB || UnresolvedCalls.count(CB))
- continue;
+ return;
if (!CB->getCalledFunction()) {
LLVM_DEBUG(dbgs() << "[AAReturnedValues] Unresolved call: " << *CB
<< "\n");
UnresolvedCalls.insert(CB);
- continue;
+ return;
}
// TODO: use the function scope once we have call site AAReturnedValues.
@@ -1037,7 +1040,7 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
LLVM_DEBUG(dbgs() << "[AAReturnedValues] Unresolved call: " << *CB
<< "\n");
UnresolvedCalls.insert(CB);
- continue;
+ return;
}
// Do not try to learn partial information. If the callee has unresolved
@@ -1045,7 +1048,7 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
auto &RetValAAUnresolvedCalls = RetValAA.getUnresolvedCalls();
if (!RetValAAUnresolvedCalls.empty()) {
UnresolvedCalls.insert(CB);
- continue;
+ return;
}
// Now check if we can track transitively returned values. If possible, thus
@@ -1067,14 +1070,14 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
}
if (Unresolved)
- continue;
+ return;
// Now track transitively returned values.
unsigned &NumRetAA = NumReturnedValuesPerKnownAA[CB];
if (NumRetAA == RetValAA.getNumReturnValues()) {
LLVM_DEBUG(dbgs() << "[AAReturnedValues] Skip call as it has not "
"changed since it was seen last\n");
- continue;
+ return;
}
NumRetAA = RetValAA.getNumReturnValues();
@@ -1093,21 +1096,32 @@ ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
continue;
} else if (isa<Constant>(RetVal)) {
// Constants are valid everywhere, we can simply take them.
- NewRVsMap[RetVal].insert(It.second.begin(), It.second.end());
+ NewRVsMap[RetVal].insert(RIs.begin(), RIs.end());
continue;
}
}
- }
+ };
+
+ for (auto &It : ReturnedValues)
+ HandleReturnValue(It.first, It.second);
+
+ // Because processing the new information can again lead to new return values
+ // we have to be careful and iterate until this iteration is complete. The
+ // idea is that we are in a stable state at the end of an update. All return
+ // values have been handled and properly categorized. We might not update
+ // again if we have not requested a non-fix attribute so we cannot "wait" for
+ // the next update to analyze a new return value.
+ while (!NewRVsMap.empty()) {
+ auto It = std::move(NewRVsMap.back());
+ NewRVsMap.pop_back();
- // To avoid modifications to the ReturnedValues map while we iterate over it
- // we kept record of potential new entries in a copy map, NewRVsMap.
- for (auto &It : NewRVsMap) {
assert(!It.second.empty() && "Entry does not add anything.");
auto &ReturnInsts = ReturnedValues[It.first];
for (ReturnInst *RI : It.second)
if (ReturnInsts.insert(RI)) {
LLVM_DEBUG(dbgs() << "[AAReturnedValues] Add new returned value "
<< *It.first << " => " << *RI << "\n");
+ HandleReturnValue(It.first, ReturnInsts);
Changed = true;
}
}
diff --git a/llvm/test/Transforms/Attributor/returned.ll b/llvm/test/Transforms/Attributor/returned.ll
index cdc67b77c097..1d14685ed1fc 100644
--- a/llvm/test/Transforms/Attributor/returned.ll
+++ b/llvm/test/Transforms/Attributor/returned.ll
@@ -1,6 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
-; RUN: opt -attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=15 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=15 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
+; RUN: opt -attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=14 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=14 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
; RUN: opt -attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM
; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM
;
@@ -1234,4 +1234,47 @@ define i32* @dont_use_const() #0 {
ret i32* %c
}
+; UTC_ARGS: --disable
+;
+; Verify we do not derive constraints for @_Z3fooP1X as if it was returning `null`.
+;
+; CHEKC-NOT: noalias
+; CHECK-NOT: align 536870912
+
+%struct.Y = type { %struct.X }
+%struct.X = type { i32 (...)** }
+
+ at _ZTI1X = external dso_local constant { i8*, i8* }, align 8
+ at _ZTI1Y = external dso_local constant { i8*, i8*, i8* }, align 8
+
+define internal i8* @_ZN1Y3barEv(%struct.Y* %this) align 2 {
+entry:
+ %0 = bitcast %struct.Y* %this to i8*
+ ret i8* %0
+}
+
+define dso_local i8* @_Z3fooP1X(%struct.X* %x) {
+entry:
+ %0 = icmp eq %struct.X* %x, null
+ br i1 %0, label %dynamic_cast.null, label %dynamic_cast.notnull
+
+dynamic_cast.notnull: ; preds = %entry
+ %1 = bitcast %struct.X* %x to i8*
+ %2 = call i8* @__dynamic_cast(i8* %1, i8* bitcast ({ i8*, i8* }* @_ZTI1X to i8*), i8* bitcast ({ i8*, i8*, i8* }* @_ZTI1Y to i8*), i64 0) #2
+ %3 = bitcast i8* %2 to %struct.Y*
+ br label %dynamic_cast.end
+
+dynamic_cast.null: ; preds = %entry
+ br label %dynamic_cast.end
+
+dynamic_cast.end: ; preds = %dynamic_cast.null, %dynamic_cast.notnull
+ %QQ5 = phi %struct.Y* [ %3, %dynamic_cast.notnull ], [ null, %dynamic_cast.null ]
+ %call = call i8* @_ZN1Y3barEv(%struct.Y* %QQ5)
+ ret i8* %call
+}
+
+declare dso_local i8* @__dynamic_cast(i8*, i8*, i8*, i64)
+
+; UTC_ARGS: --enable
+
attributes #0 = { noinline nounwind uwtable }
More information about the llvm-commits
mailing list