[VPlan][NFCI] Small code quality fixes in VPlanHCFGBuilder

[calebwat]

Addresses some code quality issues found during an internal code review. Specifically, fixing two instances where null dereferences were technically possible in VPlanHCFGBuilder.cpp, even if not in practice.

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: None (calebwat)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/134324.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp (+9-10)
• (modified) llvm/lib/Transforms/Vectorize/VPlanSLP.h (+2)

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.

[fhahn]

If the block is for a loop, then there must be a region?

[calebwat]

I added it to the condition since the body only affects the region, and dereferences it to do so. It might be more appropriate to have it in an assert instead, though.

[fhahn]

If anything, an assert should be added. FYI the code will probably go away in the near future: #129402

[calebwat]

This patch is no longer relevant since #129402 has been merged in.