Skip to content

Commit 5b92465

Browse files
authored
[LV] Fix ScalarIVSteps vplan pattern matcher, remove m_CanonicalIV() (llvm#138298)
783a846 changed VPScalarIVStepsRecipe to take 3 arguments (adding VF explicitly) instead of 2, but didn't change the corresponding pattern matcher. This matcher was only used in vputils::isHeaderMask, and no test ever reached that function with a ScalarIVSteps recipe for the value being matched -- it was always a WideCanonicalIV. So the matcher bailed out immediately before checking arguments and asserting that the number of arguments in the recipe was the same provided by the matcher. Since the constructors for ScalarIVSteps take 3 values, we should be safe to update the matcher and guard it with a dedicated gtest. m_CanonicalIV() on the other hand is removed; as a phi recipe it may not have a consistent number of arguments to match, only requiring one (the start value) when being constructed with the assumption that a second incoming value is added for the backedge later. In order to keep the matcher we would need to add multiple matchers with different numbers of arguments for it depending on what phase of vplan construction we were in, and ensure that we never reorder matcher usage vs. vplan transformation. Since the main IR PatternMatch.h doesn't contain any matchers for PHI nodes, I think we can just remove it and match via m_Specific() using the VPValue we get from Plan.getCanonicalIV().
1 parent 6a125af commit 5b92465

File tree

4 files changed

+65
-14
lines changed

4 files changed

+65
-14
lines changed

llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -461,21 +461,14 @@ m_LogicalOr(const Op0_t &Op0, const Op1_t &Op1) {
461461
return m_Select(Op0, m_True(), Op1);
462462
}
463463

464-
using VPCanonicalIVPHI_match =
465-
Recipe_match<std::tuple<>, 0, false, VPCanonicalIVPHIRecipe>;
466-
467-
inline VPCanonicalIVPHI_match m_CanonicalIV() {
468-
return VPCanonicalIVPHI_match();
469-
}
470-
471-
template <typename Op0_t, typename Op1_t>
464+
template <typename Op0_t, typename Op1_t, typename Op2_t>
472465
using VPScalarIVSteps_match =
473-
Recipe_match<std::tuple<Op0_t, Op1_t>, 0, false, VPScalarIVStepsRecipe>;
466+
TernaryRecipe_match<Op0_t, Op1_t, Op2_t, 0, false, VPScalarIVStepsRecipe>;
474467

475-
template <typename Op0_t, typename Op1_t>
476-
inline VPScalarIVSteps_match<Op0_t, Op1_t> m_ScalarIVSteps(const Op0_t &Op0,
477-
const Op1_t &Op1) {
478-
return VPScalarIVSteps_match<Op0_t, Op1_t>(Op0, Op1);
468+
template <typename Op0_t, typename Op1_t, typename Op2_t>
469+
inline VPScalarIVSteps_match<Op0_t, Op1_t, Op2_t>
470+
m_ScalarIVSteps(const Op0_t &Op0, const Op1_t &Op1, const Op2_t &Op2) {
471+
return VPScalarIVSteps_match<Op0_t, Op1_t, Op2_t>({Op0, Op1, Op2});
479472
}
480473

481474
template <typename Op0_t, typename Op1_t, typename Op2_t>

llvm/lib/Transforms/Vectorize/VPlanUtils.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ bool vputils::isHeaderMask(const VPValue *V, VPlan &Plan) {
6262

6363
if (match(V, m_ActiveLaneMask(m_VPValue(A), m_VPValue(B))))
6464
return B == Plan.getTripCount() &&
65-
(match(A, m_ScalarIVSteps(m_CanonicalIV(), m_SpecificInt(1))) ||
65+
(match(A, m_ScalarIVSteps(m_Specific(Plan.getCanonicalIV()),
66+
m_SpecificInt(1),
67+
m_Specific(&Plan.getVF()))) ||
6668
IsWideCanonicalIV(A));
6769

6870
return match(V, m_Binary<Instruction::ICmp>(m_VPValue(A), m_VPValue(B))) &&

llvm/unittests/Transforms/Vectorize/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_llvm_unittest(VectorizeTests
1212
VPlanTest.cpp
1313
VPDomTreeTest.cpp
1414
VPlanHCFGTest.cpp
15+
VPlanPatternMatchTest.cpp
1516
VPlanSlpTest.cpp
1617
VPlanVerifierTest.cpp
1718
)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//===- llvm/unittests/Transforms/Vectorize/VPlanPatternMatchTest.cpp ------===//
2+
//
3+
//
4+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
5+
// See https://llvm.org/LICENSE.txt for license information.
6+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "../lib/Transforms/Vectorize/VPlanPatternMatch.h"
11+
#include "../lib/Transforms/Vectorize/LoopVectorizationPlanner.h"
12+
#include "../lib/Transforms/Vectorize/VPlan.h"
13+
#include "../lib/Transforms/Vectorize/VPlanHelpers.h"
14+
#include "VPlanTestBase.h"
15+
#include "llvm/IR/Instruction.h"
16+
#include "llvm/IR/Instructions.h"
17+
#include "gtest/gtest.h"
18+
19+
namespace llvm {
20+
21+
namespace {
22+
using VPPatternMatchTest = VPlanTestBase;
23+
24+
TEST_F(VPPatternMatchTest, ScalarIVSteps) {
25+
VPlan &Plan = getPlan();
26+
VPBasicBlock *VPBB = Plan.createVPBasicBlock("");
27+
VPBuilder Builder(VPBB);
28+
29+
IntegerType *I64Ty = IntegerType::get(C, 64);
30+
VPValue *StartV = Plan.getOrAddLiveIn(ConstantInt::get(I64Ty, 0));
31+
auto *CanonicalIVPHI = new VPCanonicalIVPHIRecipe(StartV, DebugLoc());
32+
Builder.insert(CanonicalIVPHI);
33+
34+
VPValue *Inc = Plan.getOrAddLiveIn(ConstantInt::get(I64Ty, 1));
35+
VPValue *VF = &Plan.getVF();
36+
VPValue *Steps = Builder.createScalarIVSteps(
37+
Instruction::Add, nullptr, CanonicalIVPHI, Inc, VF, DebugLoc());
38+
39+
VPValue *Inc2 = Plan.getOrAddLiveIn(ConstantInt::get(I64Ty, 2));
40+
VPValue *Steps2 = Builder.createScalarIVSteps(
41+
Instruction::Add, nullptr, CanonicalIVPHI, Inc2, VF, DebugLoc());
42+
43+
using namespace VPlanPatternMatch;
44+
45+
ASSERT_TRUE(match(Steps, m_ScalarIVSteps(m_Specific(CanonicalIVPHI),
46+
m_SpecificInt(1), m_Specific(VF))));
47+
ASSERT_FALSE(
48+
match(Steps2, m_ScalarIVSteps(m_Specific(CanonicalIVPHI),
49+
m_SpecificInt(1), m_Specific(VF))));
50+
ASSERT_TRUE(match(Steps2, m_ScalarIVSteps(m_Specific(CanonicalIVPHI),
51+
m_SpecificInt(2), m_Specific(VF))));
52+
}
53+
54+
} // namespace
55+
} // namespace llvm

0 commit comments

Comments
 (0)