[CodeGen] Simplify LLT bitfields. NFC. #120074
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
- Put the element size field in the same place for all non-pointer types. - Put the element size and address space fields in the same place for all pointer types. - Put the number of elements and scalable fields in the same place for all vector types. This simplifies initializationa and accessor methods isScalable, getElementCount, getScalarSizeInBits and getAddressSpace.
jayfoad
commented
Dec 16, 2024
| @@ -352,44 +339,23 @@ class LLT { | |||
| /// valid encodings, SizeInBits/SizeOfElement must be larger than 0. | |||
| /// * Non-pointer scalar (isPointer == 0 && isVector == 0): | |||
| /// SizeInBits: 32; | |||
| static const constexpr BitFieldInfo ScalarSizeFieldInfo{32, 0}; | |||
| static const constexpr BitFieldInfo ScalarSizeFieldInfo{32, 29}; | |||
There was a problem hiding this comment.
The offsets are chosen to put most fields on nice-ish byte/word boundaries on little-endian hosts. I think that's the best we can do given that the fields in LLT are a mixture of real bitfields, and fields manually packed into the 61-bit RawData bitfield.
I think it would be better to manually pack all fields into a 64-bit RawData value, but that would be an orthogonal improvement.
|
@llvm/pr-subscribers-llvm-globalisel Author: Jay Foad (jayfoad) Changes
This simplifies initialization and accessor methods isScalable, Full diff: https://github.com/llvm/llvm-project/pull/120074.diff 2 Files Affected:
diff --git a/llvm/include/llvm/CodeGenTypes/LowLevelType.h b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
index 62ee28cfac99c5..06879e1f8d15b0 100644
--- a/llvm/include/llvm/CodeGenTypes/LowLevelType.h
+++ b/llvm/include/llvm/CodeGenTypes/LowLevelType.h
@@ -169,8 +169,7 @@ class LLT {
/// vector types.
constexpr bool isScalable() const {
assert(isVector() && "Expected a vector type");
- return IsPointer ? getFieldValue(PointerVectorScalableFieldInfo)
- : getFieldValue(VectorScalableFieldInfo);
+ return getFieldValue(VectorScalableFieldInfo);
}
/// Returns true if the LLT is a fixed vector. Returns false otherwise, even
@@ -183,9 +182,7 @@ class LLT {
constexpr ElementCount getElementCount() const {
assert(IsVector && "cannot get number of elements on scalar/aggregate");
- return ElementCount::get(IsPointer
- ? getFieldValue(PointerVectorElementsFieldInfo)
- : getFieldValue(VectorElementsFieldInfo),
+ return ElementCount::get(getFieldValue(VectorElementsFieldInfo),
isScalable());
}
@@ -265,25 +262,15 @@ class LLT {
}
constexpr unsigned getScalarSizeInBits() const {
- if (IsScalar)
- return getFieldValue(ScalarSizeFieldInfo);
- if (IsVector) {
- if (!IsPointer)
- return getFieldValue(VectorSizeFieldInfo);
- else
- return getFieldValue(PointerVectorSizeFieldInfo);
- }
- assert(IsPointer && "unexpected LLT");
- return getFieldValue(PointerSizeFieldInfo);
+ if (isPointerOrPointerVector())
+ return getFieldValue(PointerSizeFieldInfo);
+ return getFieldValue(ScalarSizeFieldInfo);
}
constexpr unsigned getAddressSpace() const {
- assert(RawData != 0 && "Invalid Type");
- assert(IsPointer && "cannot get address space of non-pointer type");
- if (!IsVector)
- return getFieldValue(PointerAddressSpaceFieldInfo);
- else
- return getFieldValue(PointerVectorAddressSpaceFieldInfo);
+ assert(isPointerOrPointerVector() &&
+ "cannot get address space of non-pointer type");
+ return getFieldValue(PointerAddressSpaceFieldInfo);
}
/// Returns the vector's element type. Only valid for vector types.
@@ -352,44 +339,23 @@ class LLT {
/// valid encodings, SizeInBits/SizeOfElement must be larger than 0.
/// * Non-pointer scalar (isPointer == 0 && isVector == 0):
/// SizeInBits: 32;
- static const constexpr BitFieldInfo ScalarSizeFieldInfo{32, 0};
+ static const constexpr BitFieldInfo ScalarSizeFieldInfo{32, 29};
/// * Pointer (isPointer == 1 && isVector == 0):
/// SizeInBits: 16;
/// AddressSpace: 24;
- static const constexpr BitFieldInfo PointerSizeFieldInfo{16, 0};
- static const constexpr BitFieldInfo PointerAddressSpaceFieldInfo{
- 24, PointerSizeFieldInfo[0] + PointerSizeFieldInfo[1]};
- static_assert((PointerAddressSpaceFieldInfo[0] +
- PointerAddressSpaceFieldInfo[1]) <= 61,
- "Insufficient bits to encode all data");
+ static const constexpr BitFieldInfo PointerSizeFieldInfo{16, 45};
+ static const constexpr BitFieldInfo PointerAddressSpaceFieldInfo{24, 21};
/// * Vector-of-non-pointer (isPointer == 0 && isVector == 1):
/// NumElements: 16;
/// SizeOfElement: 32;
/// Scalable: 1;
- static const constexpr BitFieldInfo VectorElementsFieldInfo{16, 0};
- static const constexpr BitFieldInfo VectorSizeFieldInfo{
- 32, VectorElementsFieldInfo[0] + VectorElementsFieldInfo[1]};
- static const constexpr BitFieldInfo VectorScalableFieldInfo{
- 1, VectorSizeFieldInfo[0] + VectorSizeFieldInfo[1]};
- static_assert((VectorSizeFieldInfo[0] + VectorSizeFieldInfo[1]) <= 61,
- "Insufficient bits to encode all data");
+ static const constexpr BitFieldInfo VectorElementsFieldInfo{16, 5};
+ static const constexpr BitFieldInfo VectorScalableFieldInfo{1, 0};
/// * Vector-of-pointer (isPointer == 1 && isVector == 1):
/// NumElements: 16;
/// SizeOfElement: 16;
/// AddressSpace: 24;
/// Scalable: 1;
- static const constexpr BitFieldInfo PointerVectorElementsFieldInfo{16, 0};
- static const constexpr BitFieldInfo PointerVectorSizeFieldInfo{
- 16,
- PointerVectorElementsFieldInfo[1] + PointerVectorElementsFieldInfo[0]};
- static const constexpr BitFieldInfo PointerVectorAddressSpaceFieldInfo{
- 24, PointerVectorSizeFieldInfo[1] + PointerVectorSizeFieldInfo[0]};
- static const constexpr BitFieldInfo PointerVectorScalableFieldInfo{
- 1, PointerVectorAddressSpaceFieldInfo[0] +
- PointerVectorAddressSpaceFieldInfo[1]};
- static_assert((PointerVectorAddressSpaceFieldInfo[0] +
- PointerVectorAddressSpaceFieldInfo[1]) <= 61,
- "Insufficient bits to encode all data");
uint64_t IsScalar : 1;
uint64_t IsPointer : 1;
@@ -422,28 +388,16 @@ class LLT {
this->IsPointer = IsPointer;
this->IsVector = IsVector;
this->IsScalar = IsScalar;
- if (IsScalar)
- RawData = maskAndShift(SizeInBits, ScalarSizeFieldInfo);
- else if (IsVector) {
- assert(EC.isVector() && "invalid number of vector elements");
- if (!IsPointer)
- RawData =
- maskAndShift(EC.getKnownMinValue(), VectorElementsFieldInfo) |
- maskAndShift(SizeInBits, VectorSizeFieldInfo) |
- maskAndShift(EC.isScalable() ? 1 : 0, VectorScalableFieldInfo);
- else
- RawData =
- maskAndShift(EC.getKnownMinValue(),
- PointerVectorElementsFieldInfo) |
- maskAndShift(SizeInBits, PointerVectorSizeFieldInfo) |
- maskAndShift(AddressSpace, PointerVectorAddressSpaceFieldInfo) |
- maskAndShift(EC.isScalable() ? 1 : 0,
- PointerVectorScalableFieldInfo);
- } else if (IsPointer)
+ if (IsPointer) {
RawData = maskAndShift(SizeInBits, PointerSizeFieldInfo) |
maskAndShift(AddressSpace, PointerAddressSpaceFieldInfo);
- else
- llvm_unreachable("unexpected LLT configuration");
+ } else {
+ RawData = maskAndShift(SizeInBits, ScalarSizeFieldInfo);
+ }
+ if (IsVector) {
+ RawData |= maskAndShift(EC.getKnownMinValue(), VectorElementsFieldInfo) |
+ maskAndShift(EC.isScalable() ? 1 : 0, VectorScalableFieldInfo);
+ }
}
public:
diff --git a/llvm/lib/CodeGenTypes/LowLevelType.cpp b/llvm/lib/CodeGenTypes/LowLevelType.cpp
index 5530a2ea952dd9..4785f2652b00e8 100644
--- a/llvm/lib/CodeGenTypes/LowLevelType.cpp
+++ b/llvm/lib/CodeGenTypes/LowLevelType.cpp
@@ -60,8 +60,3 @@ const constexpr LLT::BitFieldInfo LLT::PointerSizeFieldInfo;
const constexpr LLT::BitFieldInfo LLT::PointerAddressSpaceFieldInfo;
const constexpr LLT::BitFieldInfo LLT::VectorElementsFieldInfo;
const constexpr LLT::BitFieldInfo LLT::VectorScalableFieldInfo;
-const constexpr LLT::BitFieldInfo LLT::VectorSizeFieldInfo;
-const constexpr LLT::BitFieldInfo LLT::PointerVectorElementsFieldInfo;
-const constexpr LLT::BitFieldInfo LLT::PointerVectorScalableFieldInfo;
-const constexpr LLT::BitFieldInfo LLT::PointerVectorSizeFieldInfo;
-const constexpr LLT::BitFieldInfo LLT::PointerVectorAddressSpaceFieldInfo;
|
arsenm
approved these changes
Dec 16, 2024
tgymnich
approved these changes
Dec 16, 2024
There was a problem hiding this comment.
Looks good!
I made some ASCII art to illustrate the memory layout.
/*
--- RawData ---
63 56 47 39 31 23 15 7 0
| | | | | | | | |
|xxxxxxxx|xxxxxxxx|xxxxxxxx|xxxxxxxx|xxxxxxxx|xxxxxxxx|xxxxxxxx|xxxxxxxx|
.................................... (1)
****************** (2)
~~~~~~~~~~~~~~~~~~~~~~~~~~~ (3)
^^^^^^^^^^^^^^^^^^ (4)
@ (5)
(1) ScalarSize (2) PointerSize (3) PointerAddressSpace
(4) VectorElements (5) VectorScalable
*/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
types.
all pointer types.
all vector types.
This simplifies initialization and accessor methods isScalable,
getElementCount, getScalarSizeInBits and getAddressSpace.