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

[Attributor] Be more careful to not disturb the CG outside the SCC 

We have seen various problems when the call graph was not updated or
the updated did not succeed because it involved functions outside the
SCC. This patch adds assertions and checks to avoid accidentally
changing something outside the SCC that would impact the call graph.
It also prevents us from reanalyzing functions outside the current
SCC which could cause problems on its own. Note that the transformations
we do might cause the CG to be "more precise" but the original one would
always be a super set of the most precise one. Since the call graph is
by nature an approximation, it is good enough to have a super set of all
call edges.
This commit is contained in:
Johannes Doerfert 2021-05-15 20:19:11 -05:00
parent c65bb760df
commit 1ba2929bb8
8 changed files with 52 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
llvm/lib/Transforms/IPO/Attributor.cpp
View File

@ -17,6 +17,7 @@
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/Analysis/InlineCost.h"
@ -27,7 +28,9 @@
#include "llvm/IR/Attributes.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instruction.h"
#include "llvm/IR/NoFolder.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/IR/Verifier.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/Casting.h"
@ -1267,16 +1270,19 @@ ChangeStatus Attributor::cleanupIR() {
// not delete.
if (isa<ReturnInst>(U->getUser()))
if (auto *CI = dyn_cast<CallInst>(OldV->stripPointerCasts()))
if (CI->isMustTailCall() && !ToBeDeletedInsts.count(CI))
if (CI->isMustTailCall() &&
(!ToBeDeletedInsts.count(CI) || !isRunOn(*CI->getCaller())))
continue;
// Do not perform call graph altering changes outside the SCC.
if (auto *CB = dyn_cast<CallBase>(U->getUser()))
if (CB->isCallee(U) && !isRunOn(*CB->getCaller()))
continue;
LLVM_DEBUG(dbgs() << "Use " << *NewV << " in " << *U->getUser()
<< " instead of " << *OldV << "\n");
U->set(NewV);
// Do not modify call instructions outside the SCC.
if (auto *CB = dyn_cast<CallBase>(OldV))
if (!Functions.count(CB->getCaller()))
continue;
if (Instruction *I = dyn_cast<Instruction>(OldV)) {
CGModifiedFunctions.insert(I->getFunction());
if (!isa<PHINode>(I) && !ToBeDeletedInsts.count(I) &&
@ -1305,6 +1311,8 @@ ChangeStatus Attributor::cleanupIR() {
}
for (auto &V : InvokeWithDeadSuccessor)
if (InvokeInst *II = dyn_cast_or_null<InvokeInst>(V)) {
assert(isRunOn(*II->getFunction()) &&
"Cannot replace an invoke outside the current SCC!");
bool UnwindBBIsDead = II->hasFnAttr(Attribute::NoUnwind);
bool NormalBBIsDead = II->hasFnAttr(Attribute::NoReturn);
bool Invoke2CallAllowed =
@ -1329,17 +1337,24 @@ ChangeStatus Attributor::cleanupIR() {
}
}
for (Instruction *I : TerminatorsToFold) {
if (!isRunOn(*I->getFunction()))
continue;
CGModifiedFunctions.insert(I->getFunction());
ConstantFoldTerminator(I->getParent());
}
for (auto &V : ToBeChangedToUnreachableInsts)
if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {
if (!isRunOn(*I->getFunction()))
continue;
CGModifiedFunctions.insert(I->getFunction());
changeToUnreachable(I, /* UseLLVMTrap */ false);
}
for (auto &V : ToBeDeletedInsts) {
if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {
if (auto *CB = dyn_cast<CallBase>(I))
if (CB->isMustTailCall() && !isRunOn(*I->getFunction()))
continue;
I->dropDroppableUses();
CGModifiedFunctions.insert(I->getFunction());
if (!I->getType()->isVoidTy())
@ -1351,8 +1366,9 @@ ChangeStatus Attributor::cleanupIR() {
}
}
LLVM_DEBUG(dbgs() << "[Attributor] DeadInsts size: " << DeadInsts.size()
<< "\n");
llvm::erase_if(DeadInsts, [&](WeakTrackingVH I) {
return !I || !isRunOn(*cast<Instruction>(I)->getFunction());
});
LLVM_DEBUG({
dbgs() << "[Attributor] DeadInsts size: " << DeadInsts.size() << "\n";
@ -1367,6 +1383,8 @@ ChangeStatus Attributor::cleanupIR() {
SmallVector<BasicBlock *, 8> ToBeDeletedBBs;
ToBeDeletedBBs.reserve(NumDeadBlocks);
for (BasicBlock *BB : ToBeDeletedBlocks) {
assert(isRunOn(*BB->getParent()) &&
"Cannot delete a block outside the current SCC!");
CGModifiedFunctions.insert(BB->getParent());
ToBeDeletedBBs.push_back(BB);
}
@ -1382,7 +1400,7 @@ ChangeStatus Attributor::cleanupIR() {
ChangeStatus ManifestChange = rewriteFunctionSignatures(CGModifiedFunctions);
for (Function *Fn : CGModifiedFunctions)
if (!ToBeDeletedFunctions.count(Fn))
if (!ToBeDeletedFunctions.count(Fn) && Functions.count(Fn))
CGUpdater.reanalyzeFunction(*Fn);
for (Function *Fn : ToBeDeletedFunctions) {
@ -1746,6 +1764,7 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
// Create the new function body and insert it into the module.
Function *NewFn = Function::Create(NewFnTy, OldFn->getLinkage(),
OldFn->getAddressSpace(), "");
Functions.insert(NewFn);
OldFn->getParent()->getFunctionList().insert(OldFn->getIterator(), NewFn);
NewFn->takeName(OldFn);
NewFn->copyAttributesFrom(OldFn);

1
llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll
View File

@ -26,6 +26,7 @@ define internal i32 @callee(i1 %C, i32* %A) {
; IS__CGSCC____-LABEL: define {{[^@]+}}@callee
; IS__CGSCC____-SAME: (i32* nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A:%.*]]) #[[ATTR0:[0-9]+]] {
; IS__CGSCC____-NEXT: entry:
; IS__CGSCC____-NEXT: [[A_0:%.*]] = load i32, i32* [[A]], align 4
; IS__CGSCC____-NEXT: br label [[F:%.*]]
; IS__CGSCC____: T:
; IS__CGSCC____-NEXT: unreachable

2
llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
View File

@ -74,6 +74,8 @@ define i64 @fn2c() {
; IS__CGSCC____-LABEL: define {{[^@]+}}@fn2c
; IS__CGSCC____-SAME: () #[[ATTR0]] {
; IS__CGSCC____-NEXT: entry:
; IS__CGSCC____-NEXT: [[CONV:%.*]] = sext i32 undef to i64
; IS__CGSCC____-NEXT: [[ADD:%.*]] = add i64 42, [[CONV]]
; IS__CGSCC____-NEXT: [[CALL2:%.*]] = call i64 @fn1(i64 noundef 42) #[[ATTR1]]
; IS__CGSCC____-NEXT: ret i64 [[CALL2]]
;

3
llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
View File

@ -8,7 +8,9 @@
declare i32 @external()
; FIXME: We should not return undef here.
define i8* @start(i8 %v) {
;
; IS__TUNIT____-LABEL: define {{[^@]+}}@start
; IS__TUNIT____-SAME: (i8 [[V:%.*]]) {
; IS__TUNIT____-NEXT: [[C1:%.*]] = icmp eq i8 [[V]], 0
@ -52,7 +54,6 @@ false:
br i1 %c2, label %c2_true, label %c2_false
c2_true:
%ca1 = musttail call i8* @no_side_effects(i8 %v)
; FIXME: zap this call
ret i8* %ca1
c2_false:
%ca2 = musttail call i8* @dont_zap_me(i8 %v)

2
llvm/test/Transforms/Attributor/nodelete.ll
View File

@ -29,7 +29,7 @@ entry:
define internal i64 @f2(%"a"* %this) align 2 {
; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC____-LABEL: define {{[^@]+}}@f2
; IS__CGSCC____-SAME: (%a* nofree noundef nonnull align 8 dereferenceable(8) [[THIS:%.*]]) #[[ATTR0]] align 2 {
; IS__CGSCC____-SAME: (%a* noalias nocapture nofree noundef nonnull readnone align 8 dereferenceable(8) [[THIS:%.*]]) #[[ATTR0]] align 2 {
; IS__CGSCC____-NEXT: entry:
; IS__CGSCC____-NEXT: [[THIS_ADDR:%.*]] = alloca %a*, align 8
; IS__CGSCC____-NEXT: store %a* [[THIS]], %a** [[THIS_ADDR]], align 8

3
llvm/test/Transforms/Attributor/potential.ll
View File

@ -329,6 +329,8 @@ define internal i32 @return2or4(i32 %c) {
; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC____-LABEL: define {{[^@]+}}@return2or4
; IS__CGSCC____-SAME: (i32 [[C:%.*]]) #[[ATTR0]] {
; IS__CGSCC____-NEXT: [[CMP:%.*]] = icmp eq i32 undef, 0
; IS__CGSCC____-NEXT: [[RET:%.*]] = select i1 undef, i32 2, i32 4
; IS__CGSCC____-NEXT: ret i32 undef
;
%cmp = icmp eq i32 %c, 0
@ -532,6 +534,7 @@ define i1 @potential_test10(i32 %c) {
; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC____-LABEL: define {{[^@]+}}@potential_test10
; IS__CGSCC____-SAME: (i32 [[C:%.*]]) #[[ATTR0]] {
; IS__CGSCC____-NEXT: [[CMP:%.*]] = icmp eq i32 undef, 0
; IS__CGSCC____-NEXT: ret i1 false
;
%ret = call i32 @may_return_undef(i32 %c)

11
llvm/test/Transforms/Attributor/range.ll
View File

@ -612,10 +612,11 @@ define void @f1(i32){
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@f1
; IS__CGSCC_NPM-SAME: (i32 [[TMP0:%.*]]) #[[ATTR1]] {
; IS__CGSCC_NPM-NEXT: br label [[TMP3:%.*]]
; IS__CGSCC_NPM: 2:
; IS__CGSCC_NPM-NEXT: unreachable
; IS__CGSCC_NPM-NEXT: [[TMP2:%.*]] = icmp sgt i32 10, 15
; IS__CGSCC_NPM-NEXT: br i1 false, label [[TMP3:%.*]], label [[TMP4:%.*]]
; IS__CGSCC_NPM: 3:
; IS__CGSCC_NPM-NEXT: unreachable
; IS__CGSCC_NPM: 4:
; IS__CGSCC_NPM-NEXT: ret void
;
%2 = tail call i32 @r1(i32 %0)
@ -1932,11 +1933,13 @@ define internal i32 @cast_and_return(i1 %c) {
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@cast_and_return
; IS__CGSCC_OPM-SAME: (i1 [[C:%.*]]) #[[ATTR2]] {
; IS__CGSCC_OPM-NEXT: [[RET:%.*]] = zext i1 undef to i32
; IS__CGSCC_OPM-NEXT: ret i32 undef
;
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@cast_and_return
; IS__CGSCC_NPM-SAME: (i1 [[C:%.*]]) #[[ATTR1]] {
; IS__CGSCC_NPM-NEXT: [[RET:%.*]] = zext i1 undef to i32
; IS__CGSCC_NPM-NEXT: ret i32 undef
;
%ret = zext i1 %c to i32
@ -2280,11 +2283,13 @@ define internal i1 @is_less_than_100_2(i32 %c) {
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@is_less_than_100_2
; IS__CGSCC_OPM-SAME: (i32 noundef [[C:%.*]]) #[[ATTR2]] {
; IS__CGSCC_OPM-NEXT: [[CMP:%.*]] = icmp slt i32 [[C]], 100
; IS__CGSCC_OPM-NEXT: ret i1 true
;
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@is_less_than_100_2
; IS__CGSCC_NPM-SAME: (i32 noundef [[C:%.*]]) #[[ATTR1]] {
; IS__CGSCC_NPM-NEXT: [[CMP:%.*]] = icmp slt i32 [[C]], 100
; IS__CGSCC_NPM-NEXT: ret i1 true
;
%cmp = icmp slt i32 %c, 100

9
llvm/test/Transforms/Attributor/value-simplify.ll
View File

@ -369,7 +369,8 @@ define internal i32 @ipccp4ib(i32 %a) {
; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
; IS__CGSCC____-LABEL: define {{[^@]+}}@ipccp4ib
; IS__CGSCC____-SAME: () #[[ATTR1]] {
; IS__CGSCC____-NEXT: br label [[T:%.*]]
; IS__CGSCC____-NEXT: [[C:%.*]] = icmp eq i32 7, 7
; IS__CGSCC____-NEXT: br i1 true, label [[T:%.*]], label [[F:%.*]]
; IS__CGSCC____: t:
; IS__CGSCC____-NEXT: ret i32 undef
; IS__CGSCC____: f:
@ -980,9 +981,12 @@ define internal i1 @undef_then_1(i1 %c, i32 %i32A, i32 %i32B) {
; IS__CGSCC_OPM: Function Attrs: nofree nosync nounwind readnone willreturn
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@undef_then_1
; IS__CGSCC_OPM-SAME: (i1 [[C:%.*]]) #[[ATTR6:[0-9]+]] {
; IS__CGSCC_OPM-NEXT: [[CMP1:%.*]] = icmp eq i32 1, 1
; IS__CGSCC_OPM-NEXT: [[CMP2:%.*]] = icmp eq i1 [[CMP1]], false
; IS__CGSCC_OPM-NEXT: [[OR:%.*]] = or i1 false, [[C]]
; IS__CGSCC_OPM-NEXT: br i1 [[OR]], label [[A:%.*]], label [[B:%.*]]
; IS__CGSCC_OPM: a:
; IS__CGSCC_OPM-NEXT: [[R2:%.*]] = call i1 @undef_then_1(i1 noundef false) #[[ATTR5]]
; IS__CGSCC_OPM-NEXT: ret i1 undef
; IS__CGSCC_OPM: b:
; IS__CGSCC_OPM-NEXT: ret i1 undef
@ -990,9 +994,12 @@ define internal i1 @undef_then_1(i1 %c, i32 %i32A, i32 %i32B) {
; IS__CGSCC_NPM: Function Attrs: nofree nosync nounwind readnone willreturn
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@undef_then_1
; IS__CGSCC_NPM-SAME: (i1 [[C:%.*]]) #[[ATTR5:[0-9]+]] {
; IS__CGSCC_NPM-NEXT: [[CMP1:%.*]] = icmp eq i32 1, 1
; IS__CGSCC_NPM-NEXT: [[CMP2:%.*]] = icmp eq i1 [[CMP1]], false
; IS__CGSCC_NPM-NEXT: [[OR:%.*]] = or i1 false, [[C]]
; IS__CGSCC_NPM-NEXT: br i1 [[OR]], label [[A:%.*]], label [[B:%.*]]
; IS__CGSCC_NPM: a:
; IS__CGSCC_NPM-NEXT: [[R2:%.*]] = call i1 @undef_then_1(i1 noundef false) #[[ATTR4]]
; IS__CGSCC_NPM-NEXT: ret i1 undef
; IS__CGSCC_NPM: b:
; IS__CGSCC_NPM-NEXT: ret i1 undef