From 8cfe2f2ea9c827ffb6c7619615c7bf52b80cc785 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 3 Feb 2025 19:13:33 +0800 Subject: [PATCH 1/3] [LV] Update incoming blocks in VPWidenPHIRecipe in reassociateBlocks This is extracted from #118638 After c7ebe4f we will crash in fixNonInductionPHIs if we use a VPWidenPHIRecipe with the vector preheader as an incoming block, because the phi will reference the old non-IRBB vector preheader. This fixes this by updating VPBlockUtils::reassociateBlocks to update any VPWidenPHIRecipes's incoming blocks. This assumes that if the VPWidenPHIRecipe is in a VPRegionBlock, it's in the entry block, and that we are replacing a VPBasicBlock with another VPBasicBlock. --- llvm/lib/Transforms/Vectorize/VPlan.h | 5 +++ llvm/lib/Transforms/Vectorize/VPlanUtils.h | 13 ++++++- .../Transforms/Vectorize/VPlanTest.cpp | 39 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h index 459222234bc37..cb58abec61c11 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -2347,6 +2347,11 @@ class VPWidenPHIRecipe : public VPSingleDefRecipe { /// Returns the \p I th incoming VPBasicBlock. VPBasicBlock *getIncomingBlock(unsigned I) { return IncomingBlocks[I]; } + /// Set the \p I th incoming VPBasicBlock to \p IncomingBlock. + void setIncomingBlock(unsigned I, VPBasicBlock *IncomingBlock) { + IncomingBlocks[I] = IncomingBlock; + } + /// Returns the \p I th incoming VPValue. VPValue *getIncomingValue(unsigned I) { return getOperand(I); } }; diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.h b/llvm/lib/Transforms/Vectorize/VPlanUtils.h index 6ddb88308955f..ddbfc50ed9615 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanUtils.h +++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.h @@ -169,8 +169,19 @@ class VPBlockUtils { 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())) + for (auto *Succ : to_vector(Old->getSuccessors())) { Succ->replacePredecessor(Old, New); + + // Replace any references to Old in widened phi incoming blocks. + while (auto *Region = dyn_cast(Succ)) + Succ = Region->getEntry(); + + for (auto &R : *cast(Succ)) + if (auto *WidenPhiR = dyn_cast(&R)) + for (unsigned I = 0; I < WidenPhiR->getNumOperands(); I++) + if (WidenPhiR->getIncomingBlock(I) == Old) + WidenPhiR->setIncomingBlock(I, cast(New)); + } New->setPredecessors(Old->getPredecessors()); New->setSuccessors(Old->getSuccessors()); Old->clearPredecessors(); diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp index e7987a95f1ca2..09f111ec31a4d 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp @@ -659,6 +659,45 @@ TEST_F(VPBasicBlockTest, TraversingIteratorTest) { } } +TEST_F(VPBasicBlockTest, reassociateBlocks) { + { + VPlan &Plan = getPlan(); + VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("VPBB1"); + VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("VPBB2"); + VPBlockUtils::connectBlocks(VPBB1, VPBB2); + + auto *WidenPhi = new VPWidenPHIRecipe(nullptr); + IntegerType *Int32 = IntegerType::get(C, 32); + VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1)); + WidenPhi->addIncoming(Val, VPBB1); + VPBB2->appendRecipe(WidenPhi); + + VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew"); + VPBlockUtils::reassociateBlocks(VPBB1, VPBBNew); + EXPECT_EQ(VPBB2->getSinglePredecessor(), VPBBNew); + EXPECT_EQ(WidenPhi->getIncomingBlock(0), VPBBNew); + } + + { + VPlan &Plan = getPlan(); + VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("VPBB1"); + VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("VPBB2"); + VPRegionBlock *R1 = Plan.createVPRegionBlock(VPBB2, VPBB2, "R1"); + VPBlockUtils::connectBlocks(VPBB1, R1); + + auto *WidenPhi = new VPWidenPHIRecipe(nullptr); + IntegerType *Int32 = IntegerType::get(C, 32); + VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1)); + WidenPhi->addIncoming(Val, VPBB1); + VPBB2->appendRecipe(WidenPhi); + + VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew"); + VPBlockUtils::reassociateBlocks(VPBB1, VPBBNew); + EXPECT_EQ(R1->getSinglePredecessor(), VPBBNew); + EXPECT_EQ(WidenPhi->getIncomingBlock(0), VPBBNew); + } +} + #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) TEST_F(VPBasicBlockTest, print) { VPInstruction *TC = new VPInstruction(Instruction::Add, {}); From 13e49ef452be956396fb43cf31803086fbbc4847 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 3 Feb 2025 21:00:29 +0800 Subject: [PATCH 2/3] Use Succ->getEntryBasicBlock()->phis(), add comments --- llvm/lib/Transforms/Vectorize/VPlanUtils.h | 5 +---- llvm/unittests/Transforms/Vectorize/VPlanTest.cpp | 4 ++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.h b/llvm/lib/Transforms/Vectorize/VPlanUtils.h index ddbfc50ed9615..ac5e1978fcfbe 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanUtils.h +++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.h @@ -173,10 +173,7 @@ class VPBlockUtils { Succ->replacePredecessor(Old, New); // Replace any references to Old in widened phi incoming blocks. - while (auto *Region = dyn_cast(Succ)) - Succ = Region->getEntry(); - - for (auto &R : *cast(Succ)) + for (auto &R : Succ->getEntryBasicBlock()->phis()) if (auto *WidenPhiR = dyn_cast(&R)) for (unsigned I = 0; I < WidenPhiR->getNumOperands(); I++) if (WidenPhiR->getIncomingBlock(I) == Old) diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp index 09f111ec31a4d..0b57f8084e5f4 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp @@ -661,6 +661,8 @@ TEST_F(VPBasicBlockTest, TraversingIteratorTest) { TEST_F(VPBasicBlockTest, reassociateBlocks) { { + // Ensure that when we reassociate a basic block, we make sure to update any + // references to it in VPWidenPHIRecipes' incoming blocks. VPlan &Plan = getPlan(); VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("VPBB1"); VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("VPBB2"); @@ -679,6 +681,8 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) { } { + // Ensure that we update VPWidenPHIRecipes that are nested inside a + // VPRegionBlock. VPlan &Plan = getPlan(); VPBasicBlock *VPBB1 = Plan.createVPBasicBlock("VPBB1"); VPBasicBlock *VPBB2 = Plan.createVPBasicBlock("VPBB2"); From 95e4fb112ec50a1c96b603b7446574f9b5d17851 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Mon, 3 Feb 2025 23:07:03 +0800 Subject: [PATCH 3/3] Empty commit to try to wake up GitHub