[llvm] [NFC][LoopVectorize] Make replaceVPBBWithIRVPBB more efficient (PR #111514)
David Sherwood via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 15 02:19:04 PDT 2024
https://github.com/david-arm updated https://github.com/llvm/llvm-project/pull/111514
>From 389ab09bdeda02b7b91237dc2812572fb86f5919 Mon Sep 17 00:00:00 2001
From: David Sherwood <david.sherwood at arm.com>
Date: Tue, 8 Oct 2024 09:28:27 +0000
Subject: [PATCH 1/4] [NFC][LoopVectorize] Make replaceVPBBWithIRVPBB more
efficient
In replaceVPBBWithIRVPBB we spend time erasing and appending
predecessors and successors from a list, when all we really have
to do is replace the old with the new. Not only is this more
efficient, but it also preserves the ordering of successors and
predecessors. This is something which may become important for
vectorising early exit loops (see PR #88385), since a
VPIRInstruction is the wrapper for a live-out phi with extra
operands that map to the incoming block according to the block's
predecessor.
---
llvm/lib/Transforms/Vectorize/VPlan.cpp | 14 ++++++++------
llvm/lib/Transforms/Vectorize/VPlan.h | 20 ++++++++++++++++++++
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 5e3a6388094940..530c55bfe8e74f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1005,12 +1005,14 @@ static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) {
R.moveBefore(*IRVPBB, IRVPBB->end());
}
VPBlockBase *PredVPBB = VPBB->getSinglePredecessor();
- VPBlockUtils::disconnectBlocks(PredVPBB, VPBB);
- VPBlockUtils::connectBlocks(PredVPBB, IRVPBB);
- for (auto *Succ : to_vector(VPBB->getSuccessors())) {
- VPBlockUtils::connectBlocks(IRVPBB, Succ);
- VPBlockUtils::disconnectBlocks(VPBB, Succ);
- }
+ PredVPBB->replaceSuccessor(VPBB, IRVPBB);
+ IRVPBB->setPredecessors({PredVPBB});
+ for (auto *Succ : to_vector(VPBB->getSuccessors()))
+ Succ->replacePredecessor(VPBB, IRVPBB);
+ IRVPBB->setSuccessors(VPBB->getSuccessors());
+
+ VPBB->clearSuccessors();
+ VPBB->clearPredecessors();
delete VPBB;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 8c5246d613c13d..88cb8e47cc9278 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -556,6 +556,26 @@ class VPBlockBase {
return getEnclosingBlockWithPredecessors()->getSinglePredecessor();
}
+ /// This function replaces one predecessor with another, useful when
+ /// trying to replace an old block in the CFG with a new one.
+ void replacePredecessor(VPBlockBase *Old, VPBlockBase *New) {
+ auto I = find(Predecessors, Old);
+ assert(I != Predecessors.end());
+ assert(Old->getParent() == New->getParent() &&
+ "replaced predecessor must have the same parent");
+ *I = New;
+ }
+
+ /// This function replaces one successor with another, useful when
+ /// trying to replace an old block in the CFG with a new one.
+ void replaceSuccessor(VPBlockBase *Old, VPBlockBase *New) {
+ auto I = find(Successors, Old);
+ assert(I != Successors.end());
+ assert(Old->getParent() == New->getParent() &&
+ "replaced successor must have the same parent");
+ *I = New;
+ }
+
/// Set a given VPBlockBase \p Successor as the single successor of this
/// VPBlockBase. This VPBlockBase is not added as predecessor of \p Successor.
/// This VPBlockBase must have no successors.
>From 6c4522402333c4d52f83b3e6851499dbc4513c5d Mon Sep 17 00:00:00 2001
From: David Sherwood <david.sherwood at arm.com>
Date: Fri, 11 Oct 2024 08:53:35 +0000
Subject: [PATCH 2/4] Address review comments
* Add new VPBlockUtils::reassociateBlocks that replaces all
associations to Old with New.
* Removed code to clear successors/predecessors in the deleted
block.
---
llvm/lib/Transforms/Vectorize/VPlan.cpp | 11 ++++-------
llvm/lib/Transforms/Vectorize/VPlan.h | 9 +++++++++
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 530c55bfe8e74f..eea2cc1645cadd 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1004,15 +1004,12 @@ static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) {
assert(!R.isPhi() && "Tried to move phi recipe to end of block");
R.moveBefore(*IRVPBB, IRVPBB->end());
}
- VPBlockBase *PredVPBB = VPBB->getSinglePredecessor();
- PredVPBB->replaceSuccessor(VPBB, IRVPBB);
- IRVPBB->setPredecessors({PredVPBB});
- for (auto *Succ : to_vector(VPBB->getSuccessors()))
- Succ->replacePredecessor(VPBB, IRVPBB);
+
+ VPBlockUtils::reassociateBlocks(VPBB, IRVPBB);
+ IRVPBB->setPredecessors(VPBB->getPredecessors());
IRVPBB->setSuccessors(VPBB->getSuccessors());
- VPBB->clearSuccessors();
- VPBB->clearPredecessors();
+ // Any successor/predecessor lists will be cleared on deletion.
delete VPBB;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 88cb8e47cc9278..bac411ff58c328 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -3882,6 +3882,15 @@ class VPBlockUtils {
To->removePredecessor(From);
}
+ /// Reassociate all the blocks connected to \p Old so that they now point to
+ /// \p New.
+ static void reassociateBlocks(VPBlockBase *Old, VPBlockBase *New) {
+ for (auto *Pred : to_vector(Old->getPredecessors()))
+ Pred->replaceSuccessor(Old, New);
+ for (auto *Succ : to_vector(Old->getSuccessors()))
+ Succ->replacePredecessor(Old, New);
+ }
+
/// Return an iterator range over \p Range which only includes \p BlockTy
/// blocks. The accesses are casted to \p BlockTy.
template <typename BlockTy, typename T>
>From 3da4e4eaf86eafefdd7bb37af5541a00b7327d1e Mon Sep 17 00:00:00 2001
From: David Sherwood <david.sherwood at arm.com>
Date: Mon, 14 Oct 2024 07:51:37 +0000
Subject: [PATCH 3/4] Make new replaceXYZ functions private
* Made functions private in same way as removeSuccessor and
removePredecessor so that we no longer have to worry about
maintaining a valid state of `Old`.
---
llvm/lib/Transforms/Vectorize/VPlan.h | 40 +++++++++++++--------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index bac411ff58c328..5dd443034164a4 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -440,6 +440,26 @@ class VPBlockBase {
Successors.erase(Pos);
}
+ /// This function replaces one predecessor with another, useful when
+ /// trying to replace an old block in the CFG with a new one.
+ void replacePredecessor(VPBlockBase *Old, VPBlockBase *New) {
+ auto I = find(Predecessors, Old);
+ assert(I != Predecessors.end());
+ assert(Old->getParent() == New->getParent() &&
+ "replaced predecessor must have the same parent");
+ *I = New;
+ }
+
+ /// This function replaces one successor with another, useful when
+ /// trying to replace an old block in the CFG with a new one.
+ void replaceSuccessor(VPBlockBase *Old, VPBlockBase *New) {
+ auto I = find(Successors, Old);
+ assert(I != Successors.end());
+ assert(Old->getParent() == New->getParent() &&
+ "replaced successor must have the same parent");
+ *I = New;
+ }
+
protected:
VPBlockBase(const unsigned char SC, const std::string &N)
: SubclassID(SC), Name(N) {}
@@ -556,26 +576,6 @@ class VPBlockBase {
return getEnclosingBlockWithPredecessors()->getSinglePredecessor();
}
- /// This function replaces one predecessor with another, useful when
- /// trying to replace an old block in the CFG with a new one.
- void replacePredecessor(VPBlockBase *Old, VPBlockBase *New) {
- auto I = find(Predecessors, Old);
- assert(I != Predecessors.end());
- assert(Old->getParent() == New->getParent() &&
- "replaced predecessor must have the same parent");
- *I = New;
- }
-
- /// This function replaces one successor with another, useful when
- /// trying to replace an old block in the CFG with a new one.
- void replaceSuccessor(VPBlockBase *Old, VPBlockBase *New) {
- auto I = find(Successors, Old);
- assert(I != Successors.end());
- assert(Old->getParent() == New->getParent() &&
- "replaced successor must have the same parent");
- *I = New;
- }
-
/// Set a given VPBlockBase \p Successor as the single successor of this
/// VPBlockBase. This VPBlockBase is not added as predecessor of \p Successor.
/// This VPBlockBase must have no successors.
>From d1b45da07bb6f2fb98e9e4c47a531a698726ca13 Mon Sep 17 00:00:00 2001
From: David Sherwood <david.sherwood at arm.com>
Date: Tue, 15 Oct 2024 09:18:12 +0000
Subject: [PATCH 4/4] Ensure reassociateBlocks leaves Old and New in a decent
state
* Since reassociateBlocks is a public interface we should leave
the Old and New blocks in a usable state.
---
llvm/lib/Transforms/Vectorize/VPlan.cpp | 3 ---
llvm/lib/Transforms/Vectorize/VPlan.h | 4 ++++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index eea2cc1645cadd..7ded51d9e3abd1 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1006,10 +1006,7 @@ static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) {
}
VPBlockUtils::reassociateBlocks(VPBB, IRVPBB);
- IRVPBB->setPredecessors(VPBB->getPredecessors());
- IRVPBB->setSuccessors(VPBB->getSuccessors());
- // Any successor/predecessor lists will be cleared on deletion.
delete VPBB;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 5dd443034164a4..c5b3ea1b1d2450 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -3889,6 +3889,10 @@ class VPBlockUtils {
Pred->replaceSuccessor(Old, New);
for (auto *Succ : to_vector(Old->getSuccessors()))
Succ->replacePredecessor(Old, New);
+ New->setPredecessors(Old->getPredecessors());
+ New->setSuccessors(Old->getSuccessors());
+ Old->clearPredecessors();
+ Old->clearSuccessors();
}
/// Return an iterator range over \p Range which only includes \p BlockTy
More information about the llvm-commits
mailing list