[llvm] 1ba2929 - [Attributor] Be more careful to not disturb the CG outside the SCC
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Sun May 23 21:01:15 PDT 2021
Author: Johannes Doerfert
Date: 2021-05-23T23:00:39-05:00
New Revision: 1ba2929bb82baa4f3b7e1314f166ef2bd6c5e6d0
URL: https://github.com/llvm/llvm-project/commit/1ba2929bb82baa4f3b7e1314f166ef2bd6c5e6d0
DIFF: https://github.com/llvm/llvm-project/commit/1ba2929bb82baa4f3b7e1314f166ef2bd6c5e6d0.diff
LOG: [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.
Added:
Modified:
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll
llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
llvm/test/Transforms/Attributor/nodelete.ll
llvm/test/Transforms/Attributor/potential.ll
llvm/test/Transforms/Attributor/range.ll
llvm/test/Transforms/Attributor/value-simplify.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index d49c1fc069f5..bc84b4f8b85b 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -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);
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll
index 833b784d62ea..2a74a5003b73 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll
@@ -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
diff --git a/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll b/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
index e41262281ad7..4a4c5bf8611c 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/PR16052.ll
@@ -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]]
;
diff --git a/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll b/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
index 9a498e598e5a..5ccec76faf24 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
@@ -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)
diff --git a/llvm/test/Transforms/Attributor/nodelete.ll b/llvm/test/Transforms/Attributor/nodelete.ll
index 9b66359a27dd..a214084af5e0 100644
--- a/llvm/test/Transforms/Attributor/nodelete.ll
+++ b/llvm/test/Transforms/Attributor/nodelete.ll
@@ -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
diff --git a/llvm/test/Transforms/Attributor/potential.ll b/llvm/test/Transforms/Attributor/potential.ll
index 57d529f5f19c..b8675c52f22e 100644
--- a/llvm/test/Transforms/Attributor/potential.ll
+++ b/llvm/test/Transforms/Attributor/potential.ll
@@ -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)
diff --git a/llvm/test/Transforms/Attributor/range.ll b/llvm/test/Transforms/Attributor/range.ll
index e7e7bfd869f5..d79e5c1d1f27 100644
--- a/llvm/test/Transforms/Attributor/range.ll
+++ b/llvm/test/Transforms/Attributor/range.ll
@@ -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
diff --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll
index 8659346720fd..96cce9550d25 100644
--- a/llvm/test/Transforms/Attributor/value-simplify.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify.ll
@@ -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
More information about the llvm-commits
mailing list