[llvm] [VPlan][NFCI] Small code quality fixes in VPlanHCFGBuilder and VPlanSLP (PR #134324)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 7 15:46:54 PDT 2025
https://github.com/calebwat updated https://github.com/llvm/llvm-project/pull/134324
>From 3e1ff6d19c4bebf6e2c7d69a9dce2f359e379ac5 Mon Sep 17 00:00:00 2001
From: "Watts, Caleb" <caleb.watts at intel.com>
Date: Thu, 3 Apr 2025 16:15:43 -0700
Subject: [PATCH 1/2] [VPlan][NFCI] Small code quality fixes in
VPlanHCFGBuilder and VPlanSLP
Addresses some code quality issues found during an internal code review. Specifically, marking implicitly defined copy constructor and copy assignment for VPInterleavedAccessInfo as delete in VPlanSLP.h, and fixing two instances where null dereferences were technically possible in VPlanHCFGBuilder.cpp.
---
.../Transforms/Vectorize/VPlanHCFGBuilder.cpp | 19 +++++++++----------
llvm/lib/Transforms/Vectorize/VPlanSLP.h | 2 ++
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 4b8a2420b3037..7ea37c1bb0481 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -378,6 +378,7 @@ void PlainCFGBuilder::buildPlainCFG(
VPBasicBlock *VPBB = getOrCreateVPBB(BB);
VPRegionBlock *Region = VPBB->getParent();
Loop *LoopForBB = LI->getLoopFor(BB);
+ assert(LoopForBB && "Expected block to be inside a loop");
// Set VPBB predecessors in the same order as they are in the incoming BB.
if (!isHeaderBB(BB, LoopForBB)) {
setVPBBPredsFromBB(VPBB, BB);
@@ -423,7 +424,7 @@ void PlainCFGBuilder::buildPlainCFG(
BasicBlock *IRSucc1 = BI->getSuccessor(1);
VPBasicBlock *Successor0 = getOrCreateVPBB(IRSucc0);
VPBasicBlock *Successor1 = getOrCreateVPBB(IRSucc1);
- if (BB == LoopForBB->getLoopLatch()) {
+ if (BB == LoopForBB->getLoopLatch() && Region) {
// For a latch we need to set the successor of the region rather than that
// of VPBB and it should be set to the exit, i.e., non-header successor,
// except for the top region, which is handled elsewhere.
@@ -437,15 +438,13 @@ void PlainCFGBuilder::buildPlainCFG(
// Don't connect any blocks outside the current loop except the latch for
// now. The latch is handled above.
- if (LoopForBB) {
- if (!LoopForBB->contains(IRSucc0)) {
- VPBB->setOneSuccessor(Successor1);
- continue;
- }
- if (!LoopForBB->contains(IRSucc1)) {
- VPBB->setOneSuccessor(Successor0);
- continue;
- }
+ if (!LoopForBB->contains(IRSucc0)) {
+ VPBB->setOneSuccessor(Successor1);
+ continue;
+ }
+ if (!LoopForBB->contains(IRSucc1)) {
+ VPBB->setOneSuccessor(Successor0);
+ continue;
}
VPBB->setTwoSuccessors(Successor0, Successor1);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanSLP.h b/llvm/lib/Transforms/Vectorize/VPlanSLP.h
index 93f04e6e30a6f..7f123689170ad 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanSLP.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanSLP.h
@@ -48,6 +48,8 @@ class VPInterleavedAccessInfo {
public:
VPInterleavedAccessInfo(VPlan &Plan, InterleavedAccessInfo &IAI);
+ VPInterleavedAccessInfo(const VPInterleavedAccessInfo &) = delete;
+ VPInterleavedAccessInfo &operator=(const VPInterleavedAccessInfo &) = delete;
~VPInterleavedAccessInfo() {
// Avoid releasing a pointer twice.
>From 5b7bd8ba67a4e80661190a80394b89562b4e83c3 Mon Sep 17 00:00:00 2001
From: "Watts, Caleb" <caleb.watts at intel.com>
Date: Mon, 7 Apr 2025 15:46:38 -0700
Subject: [PATCH 2/2] Remove changes to VPlanSLP.h to be put in a separate PR.
---
llvm/lib/Transforms/Vectorize/VPlanSLP.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanSLP.h b/llvm/lib/Transforms/Vectorize/VPlanSLP.h
index 7f123689170ad..93f04e6e30a6f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanSLP.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanSLP.h
@@ -48,8 +48,6 @@ class VPInterleavedAccessInfo {
public:
VPInterleavedAccessInfo(VPlan &Plan, InterleavedAccessInfo &IAI);
- VPInterleavedAccessInfo(const VPInterleavedAccessInfo &) = delete;
- VPInterleavedAccessInfo &operator=(const VPInterleavedAccessInfo &) = delete;
~VPInterleavedAccessInfo() {
// Avoid releasing a pointer twice.
More information about the llvm-commits
mailing list