[LV] Restrict widest induction type to be IntegerType (NFC) #128173
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As the name of the function suggests, convertPointerToIntegerType should return an IntegerType instead of a Type, and should only ever be called with integer or ptr type. Fix the callers getWiderType, and addInductionPhi to narrow the type of WidestIndTy to IntegerType, stripping unclear casts. While at it, rename convertPointerToIntegerType and getWiderType for clarity.
Member
|
@llvm/pr-subscribers-vectorizers Author: Ramkumar Ramachandra (artagnon) ChangesAs the name of the function suggests, convertPointerToIntegerType should return an IntegerType instead of a Type, and should only ever be called with integer or ptr type. Fix the callers getWiderType, and addInductionPhi to narrow the type of WidestIndTy to IntegerType, stripping unclear casts. While at it, rename convertPointerToIntegerType and getWiderType for clarity. Full diff: https://github.com/llvm/llvm-project/pull/128173.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
index e959d93b57275..c3a04f9373dbe 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
@@ -308,7 +308,7 @@ class LoopVectorizationLegality {
RecurrenceSet &getFixedOrderRecurrences() { return FixedOrderRecurrences; }
/// Returns the widest induction type.
- Type *getWidestInductionType() { return WidestIndTy; }
+ IntegerType *getWidestInductionType() { return WidestIndTy; }
/// Returns True if given store is a final invariant store of one of the
/// reductions found in the loop.
@@ -595,7 +595,7 @@ class LoopVectorizationLegality {
RecurrenceSet FixedOrderRecurrences;
/// Holds the widest induction type encountered.
- Type *WidestIndTy = nullptr;
+ IntegerType *WidestIndTy = nullptr;
/// Allowed outside users. This holds the variables that can be accessed from
/// outside the loop.
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 420cbc5384ce4..4177eb0fef470 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -395,24 +395,25 @@ static bool isUniformLoopNest(Loop *Lp, Loop *OuterLp) {
return true;
}
-static Type *convertPointerToIntegerType(const DataLayout &DL, Type *Ty) {
+static IntegerType *getInductionIntegerTy(const DataLayout &DL, Type *Ty) {
+ assert(Ty->isIntOrPtrTy() && "Expected integer or pointer type");
+
if (Ty->isPointerTy())
- return DL.getIntPtrType(Ty);
+ return DL.getIntPtrType(Ty->getContext(), Ty->getPointerAddressSpace());
// It is possible that char's or short's overflow when we ask for the loop's
// trip count, work around this by changing the type size.
if (Ty->getScalarSizeInBits() < 32)
return Type::getInt32Ty(Ty->getContext());
- return Ty;
+ return cast<IntegerType>(Ty);
}
-static Type *getWiderType(const DataLayout &DL, Type *Ty0, Type *Ty1) {
- Ty0 = convertPointerToIntegerType(DL, Ty0);
- Ty1 = convertPointerToIntegerType(DL, Ty1);
- if (Ty0->getScalarSizeInBits() > Ty1->getScalarSizeInBits())
- return Ty0;
- return Ty1;
+static IntegerType *getWiderInductionTy(const DataLayout &DL, Type *Ty0,
+ Type *Ty1) {
+ IntegerType *TyA = getInductionIntegerTy(DL, Ty0);
+ IntegerType *TyB = getInductionIntegerTy(DL, Ty1);
+ return (TyA->getScalarSizeInBits() > TyB->getScalarSizeInBits()) ? TyA : TyB;
}
/// Check that the instruction has outside loop users and is not an
@@ -693,11 +694,11 @@ void LoopVectorizationLegality::addInductionPhi(
const DataLayout &DL = Phi->getDataLayout();
// Get the widest type.
- if (!PhiTy->isFloatingPointTy()) {
+ if (PhiTy->isIntOrPtrTy()) {
if (!WidestIndTy)
- WidestIndTy = convertPointerToIntegerType(DL, PhiTy);
+ WidestIndTy = getInductionIntegerTy(DL, PhiTy);
else
- WidestIndTy = getWiderType(DL, PhiTy, WidestIndTy);
+ WidestIndTy = getWiderInductionTy(DL, PhiTy, WidestIndTy);
}
// Int inductions are special because we only allow one IV.
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index e8a5db28ea0a4..8c705a16d6a73 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -2353,8 +2353,8 @@ static bool isIndvarOverflowCheckKnownFalse(
// Always be conservative if we don't know the exact unroll factor.
unsigned MaxUF = UF ? *UF : Cost->TTI.getMaxInterleaveFactor(VF);
- Type *IdxTy = Cost->Legal->getWidestInductionType();
- APInt MaxUIntTripCount = cast<IntegerType>(IdxTy)->getMask();
+ IntegerType *IdxTy = Cost->Legal->getWidestInductionType();
+ APInt MaxUIntTripCount = IdxTy->getMask();
// We know the runtime overflow check is known false iff the (max) trip-count
// is known and (max) trip-count + (VF * UF) does not overflow in the type of
|
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesAs the name of the function suggests, convertPointerToIntegerType should return an IntegerType instead of a Type, and should only ever be called with integer or ptr type. Fix the callers getWiderType, and addInductionPhi to narrow the type of WidestIndTy to IntegerType, stripping unclear casts. While at it, rename convertPointerToIntegerType and getWiderType for clarity. Full diff: https://github.com/llvm/llvm-project/pull/128173.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
index e959d93b57275..c3a04f9373dbe 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
@@ -308,7 +308,7 @@ class LoopVectorizationLegality {
RecurrenceSet &getFixedOrderRecurrences() { return FixedOrderRecurrences; }
/// Returns the widest induction type.
- Type *getWidestInductionType() { return WidestIndTy; }
+ IntegerType *getWidestInductionType() { return WidestIndTy; }
/// Returns True if given store is a final invariant store of one of the
/// reductions found in the loop.
@@ -595,7 +595,7 @@ class LoopVectorizationLegality {
RecurrenceSet FixedOrderRecurrences;
/// Holds the widest induction type encountered.
- Type *WidestIndTy = nullptr;
+ IntegerType *WidestIndTy = nullptr;
/// Allowed outside users. This holds the variables that can be accessed from
/// outside the loop.
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 420cbc5384ce4..4177eb0fef470 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -395,24 +395,25 @@ static bool isUniformLoopNest(Loop *Lp, Loop *OuterLp) {
return true;
}
-static Type *convertPointerToIntegerType(const DataLayout &DL, Type *Ty) {
+static IntegerType *getInductionIntegerTy(const DataLayout &DL, Type *Ty) {
+ assert(Ty->isIntOrPtrTy() && "Expected integer or pointer type");
+
if (Ty->isPointerTy())
- return DL.getIntPtrType(Ty);
+ return DL.getIntPtrType(Ty->getContext(), Ty->getPointerAddressSpace());
// It is possible that char's or short's overflow when we ask for the loop's
// trip count, work around this by changing the type size.
if (Ty->getScalarSizeInBits() < 32)
return Type::getInt32Ty(Ty->getContext());
- return Ty;
+ return cast<IntegerType>(Ty);
}
-static Type *getWiderType(const DataLayout &DL, Type *Ty0, Type *Ty1) {
- Ty0 = convertPointerToIntegerType(DL, Ty0);
- Ty1 = convertPointerToIntegerType(DL, Ty1);
- if (Ty0->getScalarSizeInBits() > Ty1->getScalarSizeInBits())
- return Ty0;
- return Ty1;
+static IntegerType *getWiderInductionTy(const DataLayout &DL, Type *Ty0,
+ Type *Ty1) {
+ IntegerType *TyA = getInductionIntegerTy(DL, Ty0);
+ IntegerType *TyB = getInductionIntegerTy(DL, Ty1);
+ return (TyA->getScalarSizeInBits() > TyB->getScalarSizeInBits()) ? TyA : TyB;
}
/// Check that the instruction has outside loop users and is not an
@@ -693,11 +694,11 @@ void LoopVectorizationLegality::addInductionPhi(
const DataLayout &DL = Phi->getDataLayout();
// Get the widest type.
- if (!PhiTy->isFloatingPointTy()) {
+ if (PhiTy->isIntOrPtrTy()) {
if (!WidestIndTy)
- WidestIndTy = convertPointerToIntegerType(DL, PhiTy);
+ WidestIndTy = getInductionIntegerTy(DL, PhiTy);
else
- WidestIndTy = getWiderType(DL, PhiTy, WidestIndTy);
+ WidestIndTy = getWiderInductionTy(DL, PhiTy, WidestIndTy);
}
// Int inductions are special because we only allow one IV.
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index e8a5db28ea0a4..8c705a16d6a73 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -2353,8 +2353,8 @@ static bool isIndvarOverflowCheckKnownFalse(
// Always be conservative if we don't know the exact unroll factor.
unsigned MaxUF = UF ? *UF : Cost->TTI.getMaxInterleaveFactor(VF);
- Type *IdxTy = Cost->Legal->getWidestInductionType();
- APInt MaxUIntTripCount = cast<IntegerType>(IdxTy)->getMask();
+ IntegerType *IdxTy = Cost->Legal->getWidestInductionType();
+ APInt MaxUIntTripCount = IdxTy->getMask();
// We know the runtime overflow check is known false iff the (max) trip-count
// is known and (max) trip-count + (VF * UF) does not overflow in the type of
|
david-arm
reviewed
Feb 21, 2025
Contributor
david-arm
left a comment
david-arm
left a comment
There was a problem hiding this comment.
When I originally looked at this PR I was a bit confused because I'm used to the terminology of narrowing in C and C++ to mean choosing a narrower (smaller) type, i.e. one with fewer bits. Then I realised what you mean by "narrowing" here is choosing a type in the inheritance tree with fewer possible derived instances.
This is just a suggestion, but how about clarifying this a bit more in the title with something like "[LV] Restrict widest induction type to be IntegerType [NFC]"?
artagnon
changed the title
fhahn
reviewed
Feb 23, 2025
fhahn
approved these changes
Feb 24, 2025
Contributor
fhahn
left a comment
fhahn
left a comment
There was a problem hiding this comment.
LGTM, thanks for the cleanup!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the name of the function suggests, convertPointerToIntegerType should return an IntegerType instead of a Type, and should only ever be called with integer or ptr type. Fix the callers getWiderType, and addInductionPhi to narrow the type of WidestIndTy to IntegerType, stripping unclear casts. While at it, rename convertPointerToIntegerType and getWiderType for clarity.