maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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.
This commit is contained in:
Johannes Doerfert 2020-05-12 20:51:21 -05:00
parent 8aa2266fd8
commit af48351cc8
2 changed files with 75 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
View File

@ -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;
}
}

47
llvm/test/Transforms/Attributor/returned.ll
View File

@ -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 (...)** }
@_ZTI1X = external dso_local constant { i8*, i8* }, align 8
@_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 }