[llvm] [VP] Refactor VectorBuilder to avoid layering violation. NFC (PR #99276)
Mel Chen via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 18 23:10:45 PDT 2024
https://github.com/Mel-Chen updated https://github.com/llvm/llvm-project/pull/99276
>From ebb2070806e5d09b60cb7188bcd0babb075739f9 Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Tue, 16 Jul 2024 20:29:48 -0700
Subject: [PATCH 1/6] [VP] Refactor VectorBuilder to avoid layering violation.
NFC
This patch refactors the handling of reduction to eliminate layering
violations.
* Introduced `getReductionIntrinsicID` in LoopUtils.h for mapping recurrence
kinds to llvm.vector.reduce.* intrinsic IDs.
* Updated `VectorBuilder::createSimpleTargetReduction` to accept
llvm.vector.reduce.* intrinsic directly.
* New function `VPIntrinsic::getForIntrinsic` for mapping intrinsic ID to
the same functional VP intrinsic ID.
---
llvm/include/llvm/IR/IntrinsicInst.h | 3 +
llvm/include/llvm/IR/VectorBuilder.h | 5 +-
.../include/llvm/Transforms/Utils/LoopUtils.h | 4 ++
llvm/lib/IR/IntrinsicInst.cpp | 13 +++++
llvm/lib/IR/VectorBuilder.cpp | 57 ++-----------------
llvm/lib/Transforms/Utils/LoopUtils.cpp | 44 +++++++++++++-
6 files changed, 69 insertions(+), 57 deletions(-)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index fe3f92da400f8..90db2b3dcebb3 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -569,6 +569,9 @@ class VPIntrinsic : public IntrinsicInst {
/// The llvm.vp.* intrinsics for this instruction Opcode
static Intrinsic::ID getForOpcode(unsigned OC);
+ /// The llvm.vp.reduce.* intrinsics for this intrinsic.
+ static Intrinsic::ID getForIntrinsic(Intrinsic::ID);
+
// Whether \p ID is a VP intrinsic ID.
static bool isVPIntrinsic(Intrinsic::ID);
diff --git a/llvm/include/llvm/IR/VectorBuilder.h b/llvm/include/llvm/IR/VectorBuilder.h
index 6af7f6075551d..4f45834abb5bb 100644
--- a/llvm/include/llvm/IR/VectorBuilder.h
+++ b/llvm/include/llvm/IR/VectorBuilder.h
@@ -15,7 +15,6 @@
#ifndef LLVM_IR_VECTORBUILDER_H
#define LLVM_IR_VECTORBUILDER_H
-#include <llvm/Analysis/IVDescriptors.h>
#include <llvm/IR/IRBuilder.h>
#include <llvm/IR/InstrTypes.h>
#include <llvm/IR/Instruction.h>
@@ -100,11 +99,11 @@ class VectorBuilder {
const Twine &Name = Twine());
/// Emit a VP reduction intrinsic call for recurrence kind.
- /// \param Kind The kind of recurrence
+ /// \param RdxID The intrinsic id of llvm.vector.reduce.*
/// \param ValTy The type of operand which the reduction operation is
/// performed.
/// \param VecOpArray The operand list.
- Value *createSimpleTargetReduction(RecurKind Kind, Type *ValTy,
+ Value *createSimpleTargetReduction(Intrinsic::ID RdxID, Type *ValTy,
ArrayRef<Value *> VecOpArray,
const Twine &Name = Twine());
};
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index b01a447f3c28b..283889e4c991d 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -359,6 +359,10 @@ bool canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
SinkAndHoistLICMFlags &LICMFlags,
OptimizationRemarkEmitter *ORE = nullptr);
+/// Returns the llvm.vector.reduce intrinsic that corresponds to the recurrence
+/// kind.
+Intrinsic::ID getReductionIntrinsicID(RecurKind RK);
+
/// Returns the arithmetic instruction opcode used when expanding a reduction.
unsigned getArithmeticReductionInstruction(Intrinsic::ID RdxID);
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 64a14da55b15e..da3e751da832a 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -599,6 +599,19 @@ Intrinsic::ID VPIntrinsic::getForOpcode(unsigned IROPC) {
return Intrinsic::not_intrinsic;
}
+Intrinsic::ID VPIntrinsic::getForIntrinsic(Intrinsic::ID Id) {
+ switch (Id) {
+ default:
+ break;
+
+#define BEGIN_REGISTER_VP_INTRINSIC(VPID, ...) break;
+#define VP_PROPERTY_FUNCTIONAL_INTRINSIC(INTRIN) case Intrinsic::INTRIN:
+#define END_REGISTER_VP_INTRINSIC(VPID) return Intrinsic::VPID;
+#include "llvm/IR/VPIntrinsics.def"
+ }
+ return Intrinsic::not_intrinsic;
+}
+
bool VPIntrinsic::canIgnoreVectorLengthParam() const {
using namespace PatternMatch;
diff --git a/llvm/lib/IR/VectorBuilder.cpp b/llvm/lib/IR/VectorBuilder.cpp
index 5ff3082879895..8dbf25277bf5d 100644
--- a/llvm/lib/IR/VectorBuilder.cpp
+++ b/llvm/lib/IR/VectorBuilder.cpp
@@ -60,60 +60,13 @@ Value *VectorBuilder::createVectorInstruction(unsigned Opcode, Type *ReturnTy,
return createVectorInstructionImpl(VPID, ReturnTy, InstOpArray, Name);
}
-Value *VectorBuilder::createSimpleTargetReduction(RecurKind Kind, Type *ValTy,
+Value *VectorBuilder::createSimpleTargetReduction(Intrinsic::ID RdxID,
+ Type *ValTy,
ArrayRef<Value *> InstOpArray,
const Twine &Name) {
- Intrinsic::ID VPID;
- switch (Kind) {
- case RecurKind::Add:
- VPID = Intrinsic::vp_reduce_add;
- break;
- case RecurKind::Mul:
- VPID = Intrinsic::vp_reduce_mul;
- break;
- case RecurKind::And:
- VPID = Intrinsic::vp_reduce_and;
- break;
- case RecurKind::Or:
- VPID = Intrinsic::vp_reduce_or;
- break;
- case RecurKind::Xor:
- VPID = Intrinsic::vp_reduce_xor;
- break;
- case RecurKind::FMulAdd:
- case RecurKind::FAdd:
- VPID = Intrinsic::vp_reduce_fadd;
- break;
- case RecurKind::FMul:
- VPID = Intrinsic::vp_reduce_fmul;
- break;
- case RecurKind::SMax:
- VPID = Intrinsic::vp_reduce_smax;
- break;
- case RecurKind::SMin:
- VPID = Intrinsic::vp_reduce_smin;
- break;
- case RecurKind::UMax:
- VPID = Intrinsic::vp_reduce_umax;
- break;
- case RecurKind::UMin:
- VPID = Intrinsic::vp_reduce_umin;
- break;
- case RecurKind::FMax:
- VPID = Intrinsic::vp_reduce_fmax;
- break;
- case RecurKind::FMin:
- VPID = Intrinsic::vp_reduce_fmin;
- break;
- case RecurKind::FMaximum:
- VPID = Intrinsic::vp_reduce_fmaximum;
- break;
- case RecurKind::FMinimum:
- VPID = Intrinsic::vp_reduce_fminimum;
- break;
- default:
- llvm_unreachable("No VPIntrinsic for this reduction");
- }
+ auto VPID = VPIntrinsic::getForIntrinsic(RdxID);
+ assert(VPReductionIntrinsic::isVPReduction(VPID) &&
+ "No VPIntrinsic for this reduction");
return createVectorInstructionImpl(VPID, ValTy, InstOpArray, Name);
}
diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp
index 4609376a748f9..d1e7e23b40022 100644
--- a/llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -918,6 +918,44 @@ bool llvm::hasIterationCountInvariantInParent(Loop *InnerLoop,
return true;
}
+Intrinsic::ID llvm::getReductionIntrinsicID(RecurKind RK) {
+ switch (RK) {
+ default:
+ llvm_unreachable("Unexpected recurrence kind");
+ case RecurKind::Add:
+ return Intrinsic::vector_reduce_add;
+ case RecurKind::Mul:
+ return Intrinsic::vector_reduce_mul;
+ case RecurKind::And:
+ return Intrinsic::vector_reduce_and;
+ case RecurKind::Or:
+ return Intrinsic::vector_reduce_or;
+ case RecurKind::Xor:
+ return Intrinsic::vector_reduce_xor;
+ case RecurKind::FMulAdd:
+ case RecurKind::FAdd:
+ return Intrinsic::vector_reduce_fadd;
+ case RecurKind::FMul:
+ return Intrinsic::vector_reduce_fmul;
+ case RecurKind::SMax:
+ return Intrinsic::vector_reduce_smax;
+ case RecurKind::SMin:
+ return Intrinsic::vector_reduce_smin;
+ case RecurKind::UMax:
+ return Intrinsic::vector_reduce_umax;
+ case RecurKind::UMin:
+ return Intrinsic::vector_reduce_umin;
+ case RecurKind::FMax:
+ return Intrinsic::vector_reduce_fmax;
+ case RecurKind::FMin:
+ return Intrinsic::vector_reduce_fmin;
+ case RecurKind::FMaximum:
+ return Intrinsic::vector_reduce_fmaximum;
+ case RecurKind::FMinimum:
+ return Intrinsic::vector_reduce_fminimum;
+ }
+}
+
unsigned llvm::getArithmeticReductionInstruction(Intrinsic::ID RdxID) {
switch (RdxID) {
case Intrinsic::vector_reduce_fadd:
@@ -1215,12 +1253,13 @@ Value *llvm::createSimpleTargetReduction(VectorBuilder &VBuilder, Value *Src,
RecurKind Kind = Desc.getRecurrenceKind();
assert(!RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind) &&
"AnyOf reduction is not supported.");
+ Intrinsic::ID Id = getReductionIntrinsicID(Kind);
auto *SrcTy = cast<VectorType>(Src->getType());
Type *SrcEltTy = SrcTy->getElementType();
Value *Iden =
Desc.getRecurrenceIdentity(Kind, SrcEltTy, Desc.getFastMathFlags());
Value *Ops[] = {Iden, Src};
- return VBuilder.createSimpleTargetReduction(Kind, SrcTy, Ops);
+ return VBuilder.createSimpleTargetReduction(Id, SrcTy, Ops);
}
Value *llvm::createTargetReduction(IRBuilderBase &B,
@@ -1260,9 +1299,10 @@ Value *llvm::createOrderedReduction(VectorBuilder &VBuilder,
assert(Src->getType()->isVectorTy() && "Expected a vector type");
assert(!Start->getType()->isVectorTy() && "Expected a scalar type");
+ Intrinsic::ID Id = getReductionIntrinsicID(RecurKind::FAdd);
auto *SrcTy = cast<VectorType>(Src->getType());
Value *Ops[] = {Start, Src};
- return VBuilder.createSimpleTargetReduction(RecurKind::FAdd, SrcTy, Ops);
+ return VBuilder.createSimpleTargetReduction(Id, SrcTy, Ops);
}
void llvm::propagateIRFlags(Value *I, ArrayRef<Value *> VL, Value *OpValue,
>From 165f0bcf2dc7bf3f3fe98dc18c3f81e622255a67 Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Thu, 18 Jul 2024 00:30:40 -0700
Subject: [PATCH 2/6] Fix comment for getForIntrinsic.
---
llvm/include/llvm/IR/IntrinsicInst.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 90db2b3dcebb3..485dac69a6707 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -569,7 +569,7 @@ class VPIntrinsic : public IntrinsicInst {
/// The llvm.vp.* intrinsics for this instruction Opcode
static Intrinsic::ID getForOpcode(unsigned OC);
- /// The llvm.vp.reduce.* intrinsics for this intrinsic.
+ /// The llvm.vp.* intrinsics for this intrinsic ID.
static Intrinsic::ID getForIntrinsic(Intrinsic::ID);
// Whether \p ID is a VP intrinsic ID.
>From be39dca2226bff98a292c2542ff884a49c521d14 Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Thu, 18 Jul 2024 01:02:04 -0700
Subject: [PATCH 3/6] Define the behavior if passed param is VP intrinsic.
---
llvm/include/llvm/IR/IntrinsicInst.h | 5 +++--
llvm/lib/IR/IntrinsicInst.cpp | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 485dac69a6707..94c8fa092f45e 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -569,8 +569,9 @@ class VPIntrinsic : public IntrinsicInst {
/// The llvm.vp.* intrinsics for this instruction Opcode
static Intrinsic::ID getForOpcode(unsigned OC);
- /// The llvm.vp.* intrinsics for this intrinsic ID.
- static Intrinsic::ID getForIntrinsic(Intrinsic::ID);
+ /// The llvm.vp.* intrinsics for this intrinsic ID \p Id. Return \p Id if it
+ /// is already a VP intrinsic.
+ static Intrinsic::ID getForIntrinsic(Intrinsic::ID Id);
// Whether \p ID is a VP intrinsic ID.
static bool isVPIntrinsic(Intrinsic::ID);
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index da3e751da832a..1c0c015885ac4 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -600,6 +600,9 @@ Intrinsic::ID VPIntrinsic::getForOpcode(unsigned IROPC) {
}
Intrinsic::ID VPIntrinsic::getForIntrinsic(Intrinsic::ID Id) {
+ if (isVPIntrinsic(Id))
+ return Id;
+
switch (Id) {
default:
break;
>From a96aa388ba302a42b7de6db3c78f6492b3bafc2c Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Thu, 18 Jul 2024 01:07:06 -0700
Subject: [PATCH 4/6] Refine the comment for createSimpleTargetReduction.
---
llvm/include/llvm/IR/VectorBuilder.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/IR/VectorBuilder.h b/llvm/include/llvm/IR/VectorBuilder.h
index 4f45834abb5bb..dbb9f4c7336d5 100644
--- a/llvm/include/llvm/IR/VectorBuilder.h
+++ b/llvm/include/llvm/IR/VectorBuilder.h
@@ -99,7 +99,7 @@ class VectorBuilder {
const Twine &Name = Twine());
/// Emit a VP reduction intrinsic call for recurrence kind.
- /// \param RdxID The intrinsic id of llvm.vector.reduce.*
+ /// \param RdxID The intrinsic ID of llvm.vector.reduce.*
/// \param ValTy The type of operand which the reduction operation is
/// performed.
/// \param VecOpArray The operand list.
>From 741d6c0eba6b8fa6e2b0e75dd6ec332836dfc8f6 Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Thu, 18 Jul 2024 02:28:30 -0700
Subject: [PATCH 5/6] Add constexpr.
---
llvm/include/llvm/Transforms/Utils/LoopUtils.h | 2 +-
llvm/lib/IR/IntrinsicInst.cpp | 9 ++++++---
llvm/lib/Transforms/Utils/LoopUtils.cpp | 2 +-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index 283889e4c991d..56880bd4822c7 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -361,7 +361,7 @@ bool canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
/// Returns the llvm.vector.reduce intrinsic that corresponds to the recurrence
/// kind.
-Intrinsic::ID getReductionIntrinsicID(RecurKind RK);
+constexpr Intrinsic::ID getReductionIntrinsicID(RecurKind RK);
/// Returns the arithmetic instruction opcode used when expanding a reduction.
unsigned getArithmeticReductionInstruction(Intrinsic::ID RdxID);
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 1c0c015885ac4..db3b0196f66fd 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -599,14 +599,13 @@ Intrinsic::ID VPIntrinsic::getForOpcode(unsigned IROPC) {
return Intrinsic::not_intrinsic;
}
-Intrinsic::ID VPIntrinsic::getForIntrinsic(Intrinsic::ID Id) {
- if (isVPIntrinsic(Id))
+constexpr static Intrinsic::ID getForIntrinsic(Intrinsic::ID Id) {
+ if (::isVPIntrinsic(Id))
return Id;
switch (Id) {
default:
break;
-
#define BEGIN_REGISTER_VP_INTRINSIC(VPID, ...) break;
#define VP_PROPERTY_FUNCTIONAL_INTRINSIC(INTRIN) case Intrinsic::INTRIN:
#define END_REGISTER_VP_INTRINSIC(VPID) return Intrinsic::VPID;
@@ -615,6 +614,10 @@ Intrinsic::ID VPIntrinsic::getForIntrinsic(Intrinsic::ID Id) {
return Intrinsic::not_intrinsic;
}
+Intrinsic::ID VPIntrinsic::getForIntrinsic(Intrinsic::ID Id) {
+ return ::getForIntrinsic(Id);
+}
+
bool VPIntrinsic::canIgnoreVectorLengthParam() const {
using namespace PatternMatch;
diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp
index d1e7e23b40022..0abf6d77496dc 100644
--- a/llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -918,7 +918,7 @@ bool llvm::hasIterationCountInvariantInParent(Loop *InnerLoop,
return true;
}
-Intrinsic::ID llvm::getReductionIntrinsicID(RecurKind RK) {
+constexpr Intrinsic::ID llvm::getReductionIntrinsicID(RecurKind RK) {
switch (RK) {
default:
llvm_unreachable("Unexpected recurrence kind");
>From 86f96f1f8a4b4867351f0e5bfc7f7d97c1e889f7 Mon Sep 17 00:00:00 2001
From: Mel Chen <mel.chen at sifive.com>
Date: Thu, 18 Jul 2024 05:23:58 -0700
Subject: [PATCH 6/6] Add unittests
---
llvm/unittests/IR/VPIntrinsicTest.cpp | 53 +++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/llvm/unittests/IR/VPIntrinsicTest.cpp b/llvm/unittests/IR/VPIntrinsicTest.cpp
index eab2850ca4e1e..746f71acb877a 100644
--- a/llvm/unittests/IR/VPIntrinsicTest.cpp
+++ b/llvm/unittests/IR/VPIntrinsicTest.cpp
@@ -367,6 +367,59 @@ TEST_F(VPIntrinsicTest, IntrinsicIDRoundTrip) {
ASSERT_NE(FullTripCounts, 0u);
}
+/// Check that going from intrinsic to VP intrinsic and back results in the same
+/// intrinsic.
+TEST_F(VPIntrinsicTest, IntrinsicToVPRoundTrip) {
+ unsigned FullTripCounts = 0;
+ Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic + 1;
+ for (; IntrinsicID < Intrinsic::num_intrinsics; IntrinsicID++) {
+ Intrinsic::ID VPID = VPIntrinsic::getForIntrinsic(IntrinsicID);
+ // No equivalent VP intrinsic available.
+ if (VPID == Intrinsic::not_intrinsic)
+ continue;
+
+ // Return itself if passed intrinsic ID is VP intrinsic.
+ if (VPIntrinsic::isVPIntrinsic(IntrinsicID)) {
+ ASSERT_EQ(IntrinsicID, VPID);
+ continue;
+ }
+
+ std::optional<Intrinsic::ID> RoundTripIntrinsicID =
+ VPIntrinsic::getFunctionalIntrinsicIDForVP(VPID);
+ // No equivalent non-predicated intrinsic available.
+ if (!RoundTripIntrinsicID)
+ continue;
+
+ ASSERT_EQ(*RoundTripIntrinsicID, IntrinsicID);
+ ++FullTripCounts;
+ }
+ ASSERT_NE(FullTripCounts, 0u);
+}
+
+/// Check that going from VP intrinsic to equivalent non-predicated intrinsic
+/// and back results in the same intrinsic.
+TEST_F(VPIntrinsicTest, VPToNonPredIntrinsicRoundTrip) {
+ std::unique_ptr<Module> M = createVPDeclarationModule();
+ assert(M);
+
+ unsigned FullTripCounts = 0;
+ for (const auto &VPDecl : *M) {
+ auto VPID = VPDecl.getIntrinsicID();
+ std::optional<Intrinsic::ID> NonPredID =
+ VPIntrinsic::getFunctionalIntrinsicIDForVP(VPID);
+
+ // No equivalent non-predicated intrinsic available
+ if (!NonPredID)
+ continue;
+
+ Intrinsic::ID RoundTripVPID = VPIntrinsic::getForIntrinsic(*NonPredID);
+
+ ASSERT_EQ(RoundTripVPID, VPID);
+ ++FullTripCounts;
+ }
+ ASSERT_NE(FullTripCounts, 0u);
+}
+
/// Check that VPIntrinsic::getDeclarationForParams works.
TEST_F(VPIntrinsicTest, VPIntrinsicDeclarationForParams) {
std::unique_ptr<Module> M = createVPDeclarationModule();
More information about the llvm-commits
mailing list