[DataLayout] Use member initialization (NFC) #103712
Merged
Conversation
This file contains 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
This also adds a default constructor and a few uses of it.
|
@llvm/pr-subscribers-llvm-analysis Author: Sergei Barannikov (s-barannikov) ChangesThis also adds a default constructor and a few uses of it. Full diff: https://github.com/llvm/llvm-project/pull/103712.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 2b79facf1d8988..d73fe7ac300843 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -65,9 +65,6 @@ enum AlignTypeEnum {
/// Layout alignment element.
///
/// Stores the alignment data associated with a given type bit width.
-///
-/// \note The unusual order of elements in the structure attempts to reduce
-/// padding and make the structure slightly more cache friendly.
struct LayoutAlignElem {
uint32_t TypeBitWidth;
Align ABIAlign;
@@ -82,14 +79,11 @@ struct LayoutAlignElem {
/// Layout pointer alignment element.
///
/// Stores the alignment data associated with a given pointer and address space.
-///
-/// \note The unusual order of elements in the structure attempts to reduce
-/// padding and make the structure slightly more cache friendly.
struct PointerAlignElem {
+ uint32_t AddressSpace;
+ uint32_t TypeBitWidth;
Align ABIAlign;
Align PrefAlign;
- uint32_t TypeBitWidth;
- uint32_t AddressSpace;
uint32_t IndexBitWidth;
/// Initializer
@@ -115,16 +109,16 @@ class DataLayout {
MultipleOfFunctionAlign,
};
private:
- /// Defaults to false.
- bool BigEndian;
+ bool BigEndian = false;
- unsigned AllocaAddrSpace;
- unsigned ProgramAddrSpace;
- unsigned DefaultGlobalsAddrSpace;
+ unsigned AllocaAddrSpace = 0;
+ unsigned ProgramAddrSpace = 0;
+ unsigned DefaultGlobalsAddrSpace = 0;
MaybeAlign StackNaturalAlign;
MaybeAlign FunctionPtrAlign;
- FunctionPtrAlignType TheFunctionPtrAlignType;
+ FunctionPtrAlignType TheFunctionPtrAlignType =
+ FunctionPtrAlignType::Independent;
enum ManglingModeT {
MM_None,
@@ -136,24 +130,30 @@ class DataLayout {
MM_Mips,
MM_XCOFF
};
- ManglingModeT ManglingMode;
+ ManglingModeT ManglingMode = MM_None;
// FIXME: `unsigned char` truncates the value parsed by `parseSpecifier`.
SmallVector<unsigned char, 8> LegalIntWidths;
- /// Primitive type alignment data. This is sorted by type and bit
- /// width during construction.
- using AlignmentsTy = SmallVector<LayoutAlignElem, 4>;
- AlignmentsTy IntAlignments;
- AlignmentsTy FloatAlignments;
- AlignmentsTy VectorAlignments;
+ // Default primitive type specifications.
+ static const LayoutAlignElem DefaultIntSpecs[];
+ static const LayoutAlignElem DefaultFloatSpecs[];
+ static const LayoutAlignElem DefaultVectorSpecs[];
+
+ // Default pointer type specifications.
+ static const PointerAlignElem DefaultPointerSpecs[];
+
+ // Primitive type specifications. Sorted and uniqued by type bit width.
+ SmallVector<LayoutAlignElem, 6> IntAlignments;
+ SmallVector<LayoutAlignElem, 4> FloatAlignments;
+ SmallVector<LayoutAlignElem, 10> VectorAlignments;
+
+ // Pointer type specifications. Sorted and uniqued by address space number.
+ SmallVector<PointerAlignElem, 8> Pointers;
/// The string representation used to create this DataLayout
std::string StringRepresentation;
- using PointersTy = SmallVector<PointerAlignElem, 8>;
- PointersTy Pointers;
-
const PointerAlignElem &getPointerAlignElem(uint32_t AddressSpace) const;
// Struct type ABI and preferred alignments. The default spec is "a:8:64".
@@ -189,6 +189,9 @@ class DataLayout {
Error parseSpecifier(StringRef Desc);
public:
+ /// Constructs a DataLayout with default values.
+ DataLayout();
+
/// Constructs a DataLayout from a specification string.
/// WARNING: Aborts execution if the string is malformed. Use parse() instead.
explicit DataLayout(StringRef LayoutString);
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index c10b302870edbe..7ac91810255b2b 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -198,37 +198,38 @@ const char *DataLayout::getManglingComponent(const Triple &T) {
return "-m:e";
}
-static const std::pair<AlignTypeEnum, LayoutAlignElem> DefaultAlignments[] = {
- {INTEGER_ALIGN, {1, Align(1), Align(1)}}, // i1
- {INTEGER_ALIGN, {8, Align(1), Align(1)}}, // i8
- {INTEGER_ALIGN, {16, Align(2), Align(2)}}, // i16
- {INTEGER_ALIGN, {32, Align(4), Align(4)}}, // i32
- {INTEGER_ALIGN, {64, Align(4), Align(8)}}, // i64
- {FLOAT_ALIGN, {16, Align(2), Align(2)}}, // half, bfloat
- {FLOAT_ALIGN, {32, Align(4), Align(4)}}, // float
- {FLOAT_ALIGN, {64, Align(8), Align(8)}}, // double
- {FLOAT_ALIGN, {128, Align(16), Align(16)}}, // ppcf128, quad, ...
- {VECTOR_ALIGN, {64, Align(8), Align(8)}}, // v2i32, v1i64, ...
- {VECTOR_ALIGN, {128, Align(16), Align(16)}}, // v16i8, v8i16, v4i32, ...
+// Default primitive type specifications.
+// NOTE: These arrays must be sorted by type bit width.
+constexpr LayoutAlignElem DataLayout::DefaultIntSpecs[] = {
+ {1, Align::Constant<1>(), Align::Constant<1>()}, // i1:8:8
+ {8, Align::Constant<1>(), Align::Constant<1>()}, // i8:8:8
+ {16, Align::Constant<2>(), Align::Constant<2>()}, // i16:16:16
+ {32, Align::Constant<4>(), Align::Constant<4>()}, // i32:32:32
+ {64, Align::Constant<4>(), Align::Constant<8>()}, // i64:32:64
+};
+constexpr LayoutAlignElem DataLayout::DefaultFloatSpecs[] = {
+ {16, Align::Constant<2>(), Align::Constant<2>()}, // f16:16:16
+ {32, Align::Constant<4>(), Align::Constant<4>()}, // f32:32:32
+ {64, Align::Constant<8>(), Align::Constant<8>()}, // f64:64:64
+ {128, Align::Constant<16>(), Align::Constant<16>()}, // f128:128:128
+};
+constexpr LayoutAlignElem DataLayout::DefaultVectorSpecs[] = {
+ {64, Align::Constant<8>(), Align::Constant<8>()}, // v64:64:64
+ {128, Align::Constant<16>(), Align::Constant<16>()}, // v128:128:128
};
-DataLayout::DataLayout(StringRef LayoutString) {
- BigEndian = false;
- AllocaAddrSpace = 0;
- ProgramAddrSpace = 0;
- DefaultGlobalsAddrSpace = 0;
- TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
- ManglingMode = MM_None;
-
- // Default alignments
- for (const auto &[Kind, Layout] : DefaultAlignments) {
- if (Error Err = setAlignment(Kind, Layout.ABIAlign, Layout.PrefAlign,
- Layout.TypeBitWidth))
- report_fatal_error(std::move(Err));
- }
- if (Error Err = setPointerAlignmentInBits(0, Align(8), Align(8), 64, 64))
- report_fatal_error(std::move(Err));
+// Default pointer type specifications.
+constexpr PointerAlignElem DataLayout::DefaultPointerSpecs[] = {
+ {0, 64, Align::Constant<8>(), Align::Constant<8>(), 64} // p0:64:64:64:64
+};
+
+DataLayout::DataLayout()
+ : IntAlignments(ArrayRef(DefaultIntSpecs)),
+ FloatAlignments(ArrayRef(DefaultFloatSpecs)),
+ VectorAlignments(ArrayRef(DefaultVectorSpecs)),
+ Pointers(ArrayRef(DefaultPointerSpecs)) {}
+DataLayout::DataLayout(StringRef LayoutString) : DataLayout() {
if (Error Err = parseSpecifier(LayoutString))
report_fatal_error(std::move(Err));
}
@@ -277,7 +278,7 @@ bool DataLayout::operator==(const DataLayout &Other) const {
}
Expected<DataLayout> DataLayout::parse(StringRef LayoutDescription) {
- DataLayout Layout("");
+ DataLayout Layout;
if (Error Err = Layout.parseSpecifier(LayoutDescription))
return std::move(Err);
return Layout;
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 7ba5b2784f5460..704bc8d339bc57 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -73,7 +73,7 @@ template class llvm::SymbolTableListTraits<GlobalIFunc>;
Module::Module(StringRef MID, LLVMContext &C)
: Context(C), ValSymTab(std::make_unique<ValueSymbolTable>(-1)),
- ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL(""),
+ ModuleID(std::string(MID)), SourceFileName(std::string(MID)),
IsNewDbgInfoFormat(UseNewDbgInfoFormat) {
Context.addModule(this);
}
diff --git a/llvm/unittests/Analysis/ValueLatticeTest.cpp b/llvm/unittests/Analysis/ValueLatticeTest.cpp
index b456f286e0d965..1567bce2092986 100644
--- a/llvm/unittests/Analysis/ValueLatticeTest.cpp
+++ b/llvm/unittests/Analysis/ValueLatticeTest.cpp
@@ -22,7 +22,7 @@ namespace {
class ValueLatticeTest : public testing::Test {
protected:
LLVMContext Context;
- DataLayout DL = DataLayout("");
+ DataLayout DL;
};
TEST_F(ValueLatticeTest, ValueLatticeGetters) {
diff --git a/llvm/unittests/CodeGen/LowLevelTypeTest.cpp b/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
index b60d82b529fa3f..43aa4009897eeb 100644
--- a/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
+++ b/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
@@ -20,7 +20,6 @@ namespace {
TEST(LowLevelTypeTest, Token) {
LLVMContext C;
- DataLayout DL("");
const LLT TTy = LLT::token();
@@ -38,7 +37,7 @@ TEST(LowLevelTypeTest, Token) {
TEST(LowLevelTypeTest, Scalar) {
LLVMContext C;
- DataLayout DL("");
+ DataLayout DL;
for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffffU}) {
const LLT Ty = LLT::scalar(S);
@@ -70,7 +69,7 @@ TEST(LowLevelTypeTest, Scalar) {
TEST(LowLevelTypeTest, Vector) {
LLVMContext C;
- DataLayout DL("");
+ DataLayout DL;
for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffU}) {
for (auto EC :
diff --git a/llvm/unittests/IR/VectorTypesTest.cpp b/llvm/unittests/IR/VectorTypesTest.cpp
index 7ba143eeeeb81d..c592f809f7bf3e 100644
--- a/llvm/unittests/IR/VectorTypesTest.cpp
+++ b/llvm/unittests/IR/VectorTypesTest.cpp
@@ -276,7 +276,7 @@ TEST(VectorTypesTest, BaseVectorType) {
TEST(VectorTypesTest, FixedLenComparisons) {
LLVMContext Ctx;
- DataLayout DL("");
+ DataLayout DL;
Type *Int32Ty = Type::getInt32Ty(Ctx);
Type *Int64Ty = Type::getInt64Ty(Ctx);
@@ -322,7 +322,7 @@ TEST(VectorTypesTest, FixedLenComparisons) {
TEST(VectorTypesTest, ScalableComparisons) {
LLVMContext Ctx;
- DataLayout DL("");
+ DataLayout DL;
Type *Int32Ty = Type::getInt32Ty(Ctx);
Type *Int64Ty = Type::getInt64Ty(Ctx);
|
|
@llvm/pr-subscribers-llvm-ir Author: Sergei Barannikov (s-barannikov) ChangesThis also adds a default constructor and a few uses of it. Full diff: https://github.com/llvm/llvm-project/pull/103712.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 2b79facf1d8988..d73fe7ac300843 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -65,9 +65,6 @@ enum AlignTypeEnum {
/// Layout alignment element.
///
/// Stores the alignment data associated with a given type bit width.
-///
-/// \note The unusual order of elements in the structure attempts to reduce
-/// padding and make the structure slightly more cache friendly.
struct LayoutAlignElem {
uint32_t TypeBitWidth;
Align ABIAlign;
@@ -82,14 +79,11 @@ struct LayoutAlignElem {
/// Layout pointer alignment element.
///
/// Stores the alignment data associated with a given pointer and address space.
-///
-/// \note The unusual order of elements in the structure attempts to reduce
-/// padding and make the structure slightly more cache friendly.
struct PointerAlignElem {
+ uint32_t AddressSpace;
+ uint32_t TypeBitWidth;
Align ABIAlign;
Align PrefAlign;
- uint32_t TypeBitWidth;
- uint32_t AddressSpace;
uint32_t IndexBitWidth;
/// Initializer
@@ -115,16 +109,16 @@ class DataLayout {
MultipleOfFunctionAlign,
};
private:
- /// Defaults to false.
- bool BigEndian;
+ bool BigEndian = false;
- unsigned AllocaAddrSpace;
- unsigned ProgramAddrSpace;
- unsigned DefaultGlobalsAddrSpace;
+ unsigned AllocaAddrSpace = 0;
+ unsigned ProgramAddrSpace = 0;
+ unsigned DefaultGlobalsAddrSpace = 0;
MaybeAlign StackNaturalAlign;
MaybeAlign FunctionPtrAlign;
- FunctionPtrAlignType TheFunctionPtrAlignType;
+ FunctionPtrAlignType TheFunctionPtrAlignType =
+ FunctionPtrAlignType::Independent;
enum ManglingModeT {
MM_None,
@@ -136,24 +130,30 @@ class DataLayout {
MM_Mips,
MM_XCOFF
};
- ManglingModeT ManglingMode;
+ ManglingModeT ManglingMode = MM_None;
// FIXME: `unsigned char` truncates the value parsed by `parseSpecifier`.
SmallVector<unsigned char, 8> LegalIntWidths;
- /// Primitive type alignment data. This is sorted by type and bit
- /// width during construction.
- using AlignmentsTy = SmallVector<LayoutAlignElem, 4>;
- AlignmentsTy IntAlignments;
- AlignmentsTy FloatAlignments;
- AlignmentsTy VectorAlignments;
+ // Default primitive type specifications.
+ static const LayoutAlignElem DefaultIntSpecs[];
+ static const LayoutAlignElem DefaultFloatSpecs[];
+ static const LayoutAlignElem DefaultVectorSpecs[];
+
+ // Default pointer type specifications.
+ static const PointerAlignElem DefaultPointerSpecs[];
+
+ // Primitive type specifications. Sorted and uniqued by type bit width.
+ SmallVector<LayoutAlignElem, 6> IntAlignments;
+ SmallVector<LayoutAlignElem, 4> FloatAlignments;
+ SmallVector<LayoutAlignElem, 10> VectorAlignments;
+
+ // Pointer type specifications. Sorted and uniqued by address space number.
+ SmallVector<PointerAlignElem, 8> Pointers;
/// The string representation used to create this DataLayout
std::string StringRepresentation;
- using PointersTy = SmallVector<PointerAlignElem, 8>;
- PointersTy Pointers;
-
const PointerAlignElem &getPointerAlignElem(uint32_t AddressSpace) const;
// Struct type ABI and preferred alignments. The default spec is "a:8:64".
@@ -189,6 +189,9 @@ class DataLayout {
Error parseSpecifier(StringRef Desc);
public:
+ /// Constructs a DataLayout with default values.
+ DataLayout();
+
/// Constructs a DataLayout from a specification string.
/// WARNING: Aborts execution if the string is malformed. Use parse() instead.
explicit DataLayout(StringRef LayoutString);
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index c10b302870edbe..7ac91810255b2b 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -198,37 +198,38 @@ const char *DataLayout::getManglingComponent(const Triple &T) {
return "-m:e";
}
-static const std::pair<AlignTypeEnum, LayoutAlignElem> DefaultAlignments[] = {
- {INTEGER_ALIGN, {1, Align(1), Align(1)}}, // i1
- {INTEGER_ALIGN, {8, Align(1), Align(1)}}, // i8
- {INTEGER_ALIGN, {16, Align(2), Align(2)}}, // i16
- {INTEGER_ALIGN, {32, Align(4), Align(4)}}, // i32
- {INTEGER_ALIGN, {64, Align(4), Align(8)}}, // i64
- {FLOAT_ALIGN, {16, Align(2), Align(2)}}, // half, bfloat
- {FLOAT_ALIGN, {32, Align(4), Align(4)}}, // float
- {FLOAT_ALIGN, {64, Align(8), Align(8)}}, // double
- {FLOAT_ALIGN, {128, Align(16), Align(16)}}, // ppcf128, quad, ...
- {VECTOR_ALIGN, {64, Align(8), Align(8)}}, // v2i32, v1i64, ...
- {VECTOR_ALIGN, {128, Align(16), Align(16)}}, // v16i8, v8i16, v4i32, ...
+// Default primitive type specifications.
+// NOTE: These arrays must be sorted by type bit width.
+constexpr LayoutAlignElem DataLayout::DefaultIntSpecs[] = {
+ {1, Align::Constant<1>(), Align::Constant<1>()}, // i1:8:8
+ {8, Align::Constant<1>(), Align::Constant<1>()}, // i8:8:8
+ {16, Align::Constant<2>(), Align::Constant<2>()}, // i16:16:16
+ {32, Align::Constant<4>(), Align::Constant<4>()}, // i32:32:32
+ {64, Align::Constant<4>(), Align::Constant<8>()}, // i64:32:64
+};
+constexpr LayoutAlignElem DataLayout::DefaultFloatSpecs[] = {
+ {16, Align::Constant<2>(), Align::Constant<2>()}, // f16:16:16
+ {32, Align::Constant<4>(), Align::Constant<4>()}, // f32:32:32
+ {64, Align::Constant<8>(), Align::Constant<8>()}, // f64:64:64
+ {128, Align::Constant<16>(), Align::Constant<16>()}, // f128:128:128
+};
+constexpr LayoutAlignElem DataLayout::DefaultVectorSpecs[] = {
+ {64, Align::Constant<8>(), Align::Constant<8>()}, // v64:64:64
+ {128, Align::Constant<16>(), Align::Constant<16>()}, // v128:128:128
};
-DataLayout::DataLayout(StringRef LayoutString) {
- BigEndian = false;
- AllocaAddrSpace = 0;
- ProgramAddrSpace = 0;
- DefaultGlobalsAddrSpace = 0;
- TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
- ManglingMode = MM_None;
-
- // Default alignments
- for (const auto &[Kind, Layout] : DefaultAlignments) {
- if (Error Err = setAlignment(Kind, Layout.ABIAlign, Layout.PrefAlign,
- Layout.TypeBitWidth))
- report_fatal_error(std::move(Err));
- }
- if (Error Err = setPointerAlignmentInBits(0, Align(8), Align(8), 64, 64))
- report_fatal_error(std::move(Err));
+// Default pointer type specifications.
+constexpr PointerAlignElem DataLayout::DefaultPointerSpecs[] = {
+ {0, 64, Align::Constant<8>(), Align::Constant<8>(), 64} // p0:64:64:64:64
+};
+
+DataLayout::DataLayout()
+ : IntAlignments(ArrayRef(DefaultIntSpecs)),
+ FloatAlignments(ArrayRef(DefaultFloatSpecs)),
+ VectorAlignments(ArrayRef(DefaultVectorSpecs)),
+ Pointers(ArrayRef(DefaultPointerSpecs)) {}
+DataLayout::DataLayout(StringRef LayoutString) : DataLayout() {
if (Error Err = parseSpecifier(LayoutString))
report_fatal_error(std::move(Err));
}
@@ -277,7 +278,7 @@ bool DataLayout::operator==(const DataLayout &Other) const {
}
Expected<DataLayout> DataLayout::parse(StringRef LayoutDescription) {
- DataLayout Layout("");
+ DataLayout Layout;
if (Error Err = Layout.parseSpecifier(LayoutDescription))
return std::move(Err);
return Layout;
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 7ba5b2784f5460..704bc8d339bc57 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -73,7 +73,7 @@ template class llvm::SymbolTableListTraits<GlobalIFunc>;
Module::Module(StringRef MID, LLVMContext &C)
: Context(C), ValSymTab(std::make_unique<ValueSymbolTable>(-1)),
- ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL(""),
+ ModuleID(std::string(MID)), SourceFileName(std::string(MID)),
IsNewDbgInfoFormat(UseNewDbgInfoFormat) {
Context.addModule(this);
}
diff --git a/llvm/unittests/Analysis/ValueLatticeTest.cpp b/llvm/unittests/Analysis/ValueLatticeTest.cpp
index b456f286e0d965..1567bce2092986 100644
--- a/llvm/unittests/Analysis/ValueLatticeTest.cpp
+++ b/llvm/unittests/Analysis/ValueLatticeTest.cpp
@@ -22,7 +22,7 @@ namespace {
class ValueLatticeTest : public testing::Test {
protected:
LLVMContext Context;
- DataLayout DL = DataLayout("");
+ DataLayout DL;
};
TEST_F(ValueLatticeTest, ValueLatticeGetters) {
diff --git a/llvm/unittests/CodeGen/LowLevelTypeTest.cpp b/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
index b60d82b529fa3f..43aa4009897eeb 100644
--- a/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
+++ b/llvm/unittests/CodeGen/LowLevelTypeTest.cpp
@@ -20,7 +20,6 @@ namespace {
TEST(LowLevelTypeTest, Token) {
LLVMContext C;
- DataLayout DL("");
const LLT TTy = LLT::token();
@@ -38,7 +37,7 @@ TEST(LowLevelTypeTest, Token) {
TEST(LowLevelTypeTest, Scalar) {
LLVMContext C;
- DataLayout DL("");
+ DataLayout DL;
for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffffU}) {
const LLT Ty = LLT::scalar(S);
@@ -70,7 +69,7 @@ TEST(LowLevelTypeTest, Scalar) {
TEST(LowLevelTypeTest, Vector) {
LLVMContext C;
- DataLayout DL("");
+ DataLayout DL;
for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffU}) {
for (auto EC :
diff --git a/llvm/unittests/IR/VectorTypesTest.cpp b/llvm/unittests/IR/VectorTypesTest.cpp
index 7ba143eeeeb81d..c592f809f7bf3e 100644
--- a/llvm/unittests/IR/VectorTypesTest.cpp
+++ b/llvm/unittests/IR/VectorTypesTest.cpp
@@ -276,7 +276,7 @@ TEST(VectorTypesTest, BaseVectorType) {
TEST(VectorTypesTest, FixedLenComparisons) {
LLVMContext Ctx;
- DataLayout DL("");
+ DataLayout DL;
Type *Int32Ty = Type::getInt32Ty(Ctx);
Type *Int64Ty = Type::getInt64Ty(Ctx);
@@ -322,7 +322,7 @@ TEST(VectorTypesTest, FixedLenComparisons) {
TEST(VectorTypesTest, ScalableComparisons) {
LLVMContext Ctx;
- DataLayout DL("");
+ DataLayout DL;
Type *Int32Ty = Type::getInt32Ty(Ctx);
Type *Int64Ty = Type::getInt64Ty(Ctx);
|
s-barannikov
commented
Aug 14, 2024
| @@ -82,14 +79,11 @@ struct LayoutAlignElem { | |||
| /// Layout pointer alignment element. | |||
| /// | |||
| /// Stores the alignment data associated with a given pointer and address space. | |||
| /// | |||
| /// \note The unusual order of elements in the structure attempts to reduce | |||
| /// padding and make the structure slightly more cache friendly. | |||
There was a problem hiding this comment.
The comment goes back to the times when the fields were bitfields. It is no longer relevant.
nikic
reviewed
Aug 14, 2024
llvm/lib/IR/DataLayout.cpp
Outdated
| {VECTOR_ALIGN, {128, Align(16), Align(16)}}, // v16i8, v8i16, v4i32, ... | ||
| // Default primitive type specifications. | ||
| // NOTE: These arrays must be sorted by type bit width. | ||
| constexpr LayoutAlignElem DataLayout::DefaultIntSpecs[] = { |
There was a problem hiding this comment.
Do these need to be static members on DataLayout? Can we keep them local?
There was a problem hiding this comment.
In the next patch I make LayoutAlignElem a private nested class of DataLayout, which can't be used in unrelated array declaration (because it is private).
I was also thinking about make it public nested class, this could help me with unittests I was planning to improve next.
If that's OK to expose LayoutAlignElem etc I can make those arrays local to cpp file.
There was a problem hiding this comment.
I'll post a draft PR shortly.
There was a problem hiding this comment.
I think it's okay to make it public.
| struct PointerAlignElem { | ||
| uint32_t AddressSpace; | ||
| uint32_t TypeBitWidth; |
There was a problem hiding this comment.
The idea of this order change is to match how it's written in the string representation?
s-barannikov
force-pushed
the
datalayout/member-initialization
branch
from
88d2dd8 to
c5c7fde
Compare
bwendling
pushed a commit
to bwendling/llvm-project
that referenced
this pull request
Aug 15, 2024
This also adds a default constructor and a few uses of it.
bjope
reviewed
Aug 15, 2024
| {64, Align::Constant<4>(), Align::Constant<8>()}, // i64:32:64 | ||
| }; | ||
| constexpr LayoutAlignElem DataLayout::DefaultFloatSpecs[] = { | ||
| {16, Align::Constant<2>(), Align::Constant<2>()}, // f16:16:16 |
There was a problem hiding this comment.
These changes actually hard codes how many bytes that correspond to 16 bits. If the goal with this patch was to simplify life for non-8-bit-byte targets (as you wrote that you planned to do in https://discourse.llvm.org/t/rfc-on-non-8-bit-bytes-and-the-target-for-it/53455/46) then I'm confused. This currently causes some head-ache for use downstream as our old solution for converting the bit-sized alignments into bytes no longer works.
Should there perhaps be a comment somewhere saying that the alignments are specified in "number of octests"? And why is that better than specifying them in bits?
(Btw, as I mentioned in the discourse thread I wouldn't mind being added as reviewer to the patches related to non-8-bit-byte-support-improvements. This to avoid surprises like this when merging the patches that already has landed on main.)
There was a problem hiding this comment.
Sorry, I see now that the alignments weren't specified in bits before this patch either. That was just in our downstream fork.
There was a problem hiding this comment.
The end goal is indeed to simplify life for non-8-bit targets.
I'd like to do some NFC refactoring and improve test coverage before posting an RFC patch that adds the support.
I'm very sorry for the conflicts.
the alignments weren't specified in bits before this patch either.
Yep, this patch didn't change that.
Should there perhaps be a comment somewhere saying that the alignments are specified in "number of octests"? And why is that better than specifying them in bits?
Storing alignments in bits in one possible solution (and in fact I'm doing this downstream, too). It might look attractive, but it has some downsides. Let's discuss this in the discourse thread.
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.
This also adds a default constructor and a few uses of it.