[llvm] [Coroutines] Drop dead instructions more aggressively in addMustTailToCoroResumes() (PR #85271)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 15 07:23:57 PDT 2024
https://github.com/zmodem updated https://github.com/llvm/llvm-project/pull/85271
>From 97c0ff6c3b05829eb4f0b0350b2c2d000483fe06 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 14 Mar 2024 17:09:44 +0100
Subject: [PATCH 1/2] [Coroutines] Drop dead instructions more aggressively in
addMustTailToCoroResumes()
The old code used isInstructionTriviallyDead() when walking the path
from a resume call to function return to check if the call is in tail
position.
However, since the code was walking forwards it was not able to get past
instructions such as:
%gep = getelementptr inbounds i64, ptr %alloc.var, i32 0
%foo = ptrtoint ptr %gep to i64
This patch calls SimplifyInstructionsInBlock() before walking a block to
remove any dead instructions in a more rigorous fashion.
---
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 6 ++----
llvm/test/Transforms/Coroutines/coro-split-musttail7.ll | 7 ++++++-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 0fce596d3e2ca0..5d3106f157edcd 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1203,10 +1203,6 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
if (isa<BitCastInst>(I) || I->isDebugOrPseudoInst() ||
I->isLifetimeStartOrEnd())
I = I->getNextNode();
- else if (isInstructionTriviallyDead(I))
- // Duing we are in the middle of the transformation, we need to erase
- // the dead instruction manually.
- I = &*I->eraseFromParent();
else
break;
}
@@ -1245,6 +1241,7 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
}
BasicBlock *Succ = BR->getSuccessor(SuccIndex);
+ SimplifyInstructionsInBlock(Succ);
scanPHIsAndUpdateValueMap(I, Succ, ResolvedValues);
I = GetFirstValidInstruction(Succ->getFirstNonPHIOrDbgOrLifetime());
@@ -1288,6 +1285,7 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
return false;
BasicBlock *BB = SI->findCaseValue(Cond)->getCaseSuccessor();
+ SimplifyInstructionsInBlock(BB);
scanPHIsAndUpdateValueMap(I, BB, ResolvedValues);
I = GetFirstValidInstruction(BB->getFirstNonPHIOrDbgOrLifetime());
continue;
diff --git a/llvm/test/Transforms/Coroutines/coro-split-musttail7.ll b/llvm/test/Transforms/Coroutines/coro-split-musttail7.ll
index 2257d5aee473ee..241de5cfb25a7d 100644
--- a/llvm/test/Transforms/Coroutines/coro-split-musttail7.ll
+++ b/llvm/test/Transforms/Coroutines/coro-split-musttail7.ll
@@ -1,6 +1,6 @@
; Tests that sinked lifetime markers wouldn't provent optimization
; to convert a resuming call to a musttail call.
-; The difference between this and coro-split-musttail5.ll and coro-split-musttail5.ll
+; The difference between this and coro-split-musttail5.ll and coro-split-musttail6.ll
; is that this contains dead instruction generated during the transformation,
; which makes the optimization harder.
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
@@ -27,6 +27,11 @@ await.suspend:
%save2 = call token @llvm.coro.save(ptr null)
call fastcc void @fakeresume1(ptr align 8 null)
%suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
+
+ ; These (non-trivially) dead instructions are in the way.
+ %gep = getelementptr inbounds i64, ptr %alloc.var, i32 0
+ %foo = ptrtoint ptr %gep to i64
+
switch i8 %suspend2, label %exit [
i8 0, label %await.ready
i8 1, label %exit
>From e268280f2e384619b4c5b948f7ecf23ebe26b359 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Fri, 15 Mar 2024 15:23:12 +0100
Subject: [PATCH 2/2] skip any side-effect free instructions when looking for
the return
---
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 77 ++++++++------------
1 file changed, 29 insertions(+), 48 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 5d3106f157edcd..df57f9028e380f 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1197,18 +1197,6 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
assert(InitialInst->getModule());
const DataLayout &DL = InitialInst->getModule()->getDataLayout();
- auto GetFirstValidInstruction = [](Instruction *I) {
- while (I) {
- // BitCastInst wouldn't generate actual code so that we could skip it.
- if (isa<BitCastInst>(I) || I->isDebugOrPseudoInst() ||
- I->isLifetimeStartOrEnd())
- I = I->getNextNode();
- else
- break;
- }
- return I;
- };
-
auto TryResolveConstant = [&ResolvedValues](Value *V) {
auto It = ResolvedValues.find(V);
if (It != ResolvedValues.end())
@@ -1217,8 +1205,9 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
};
Instruction *I = InitialInst;
- while (I->isTerminator() || isa<CmpInst>(I)) {
+ while (true) {
if (isa<ReturnInst>(I)) {
+ assert(!cast<ReturnInst>(I)->getReturnValue());
ReplaceInstWithInst(InitialInst, I->clone());
return true;
}
@@ -1241,42 +1230,27 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
}
BasicBlock *Succ = BR->getSuccessor(SuccIndex);
- SimplifyInstructionsInBlock(Succ);
scanPHIsAndUpdateValueMap(I, Succ, ResolvedValues);
- I = GetFirstValidInstruction(Succ->getFirstNonPHIOrDbgOrLifetime());
-
+ I = Succ->getFirstNonPHIOrDbgOrLifetime();
continue;
}
- if (auto *CondCmp = dyn_cast<CmpInst>(I)) {
+ if (auto *Cmp = dyn_cast<CmpInst>(I)) {
// If the case number of suspended switch instruction is reduced to
// 1, then it is simplified to CmpInst in llvm::ConstantFoldTerminator.
- auto *BR = dyn_cast<BranchInst>(
- GetFirstValidInstruction(CondCmp->getNextNode()));
- if (!BR || !BR->isConditional() || CondCmp != BR->getCondition())
- return false;
-
- // And the comparsion looks like : %cond = icmp eq i8 %V, constant.
- // So we try to resolve constant for the first operand only since the
- // second operand should be literal constant by design.
- ConstantInt *Cond0 = TryResolveConstant(CondCmp->getOperand(0));
- auto *Cond1 = dyn_cast<ConstantInt>(CondCmp->getOperand(1));
- if (!Cond0 || !Cond1)
- return false;
-
- // Both operands of the CmpInst are Constant. So that we could evaluate
- // it immediately to get the destination.
- auto *ConstResult =
- dyn_cast_or_null<ConstantInt>(ConstantFoldCompareInstOperands(
- CondCmp->getPredicate(), Cond0, Cond1, DL));
- if (!ConstResult)
- return false;
-
- ResolvedValues[BR->getCondition()] = ConstResult;
-
- // Handle this branch in next iteration.
- I = BR;
- continue;
+ // Try to constant fold it.
+ ConstantInt *Cond0 = TryResolveConstant(Cmp->getOperand(0));
+ ConstantInt *Cond1 = TryResolveConstant(Cmp->getOperand(1));
+ if (Cond0 && Cond1) {
+ ConstantInt *Result =
+ dyn_cast_or_null<ConstantInt>(ConstantFoldCompareInstOperands(
+ Cmp->getPredicate(), Cond0, Cond1, DL));
+ if (Result) {
+ ResolvedValues[Cmp] = Result;
+ I = I->getNextNode();
+ continue;
+ }
+ }
}
if (auto *SI = dyn_cast<SwitchInst>(I)) {
@@ -1284,14 +1258,21 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
if (!Cond)
return false;
- BasicBlock *BB = SI->findCaseValue(Cond)->getCaseSuccessor();
- SimplifyInstructionsInBlock(BB);
- scanPHIsAndUpdateValueMap(I, BB, ResolvedValues);
- I = GetFirstValidInstruction(BB->getFirstNonPHIOrDbgOrLifetime());
+ BasicBlock *Succ = SI->findCaseValue(Cond)->getCaseSuccessor();
+ scanPHIsAndUpdateValueMap(I, Succ, ResolvedValues);
+ I = Succ->getFirstNonPHIOrDbgOrLifetime();
+ continue;
+ }
+
+ if (I->isDebugOrPseudoInst() || I->isLifetimeStartOrEnd() ||
+ !I->mayHaveSideEffects()) {
+ // We can skip instructions without side effects. If their values are
+ // needed, we'll notice later, e.g. when hitting a conditional branch.
+ I = I->getNextNode();
continue;
}
- return false;
+ break;
}
return false;
More information about the llvm-commits
mailing list