[llvm] [DataLayout] Remove `clear` and `reset` methods (NFC) (PR #102993)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 13 04:46:34 PDT 2024
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/102993
>From a9bb740aa828d891659baf7ed3b52c45aa3020ae Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Tue, 13 Aug 2024 05:19:05 +0300
Subject: [PATCH 1/2] [DataLayout] Remove `clear` and `reset` methods (NFC)
`clear` was never necessary as it is always called on a fresh instance
of the class or just before freeing an instance's memory.
`reset` is effectively the same as the constructor.
---
llvm/include/llvm/IR/DataLayout.h | 13 +++----------
llvm/lib/IR/DataLayout.cpp | 32 +++++++------------------------
llvm/lib/IR/Module.cpp | 4 +---
3 files changed, 11 insertions(+), 38 deletions(-)
diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 9619560f4baed3..f4b4b730bee2ae 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -185,14 +185,10 @@ class DataLayout {
/// if the string is malformed.
Error parseSpecifier(StringRef Desc);
- // Free all internal data structures.
- void clear();
-
public:
- /// Constructs a DataLayout from a specification string. See reset().
- explicit DataLayout(StringRef LayoutDescription) {
- reset(LayoutDescription);
- }
+ /// Constructs a DataLayout from a specification string.
+ /// WARNING: Aborts execution if the string is malformed. Use parse() instead.
+ explicit DataLayout(StringRef LayoutString);
DataLayout(const DataLayout &DL) { *this = DL; }
@@ -203,9 +199,6 @@ class DataLayout {
bool operator==(const DataLayout &Other) const;
bool operator!=(const DataLayout &Other) const { return !(*this == Other); }
- /// Parse a data layout string (with fallback to default values).
- void reset(StringRef LayoutDescription);
-
/// Parse a data layout string and return the layout. Return an error
/// description on failure.
static Expected<DataLayout> parse(StringRef LayoutDescription);
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 1efa61e92d2bc7..1411746a5f2f39 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -191,36 +191,30 @@ static const std::pair<AlignTypeEnum, LayoutAlignElem> DefaultAlignments[] = {
{VECTOR_ALIGN, {128, Align(16), Align(16)}}, // v16i8, v8i16, v4i32, ...
};
-void DataLayout::reset(StringRef Desc) {
- clear();
-
- LayoutMap = nullptr;
+DataLayout::DataLayout(StringRef LayoutString) {
BigEndian = false;
AllocaAddrSpace = 0;
- StackNaturalAlign.reset();
ProgramAddrSpace = 0;
DefaultGlobalsAddrSpace = 0;
- FunctionPtrAlign.reset();
TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
ManglingMode = MM_None;
- NonIntegralAddressSpaces.clear();
StructAlignment = LayoutAlignElem::get(Align(1), Align(8), 0);
// Default alignments
for (const auto &[Kind, Layout] : DefaultAlignments) {
if (Error Err = setAlignment(Kind, Layout.ABIAlign, Layout.PrefAlign,
Layout.TypeBitWidth))
- return report_fatal_error(std::move(Err));
+ report_fatal_error(std::move(Err));
}
if (Error Err = setPointerAlignmentInBits(0, Align(8), Align(8), 64, 64))
- return report_fatal_error(std::move(Err));
+ report_fatal_error(std::move(Err));
- if (Error Err = parseSpecifier(Desc))
- return report_fatal_error(std::move(Err));
+ if (Error Err = parseSpecifier(LayoutString))
+ report_fatal_error(std::move(Err));
}
DataLayout &DataLayout::operator=(const DataLayout &Other) {
- clear();
+ // Copy everything except for LayoutMap, which will be recomputed on demand.
StringRepresentation = Other.StringRepresentation;
BigEndian = Other.BigEndian;
AllocaAddrSpace = Other.AllocaAddrSpace;
@@ -716,19 +710,7 @@ class StructLayoutMap {
} // end anonymous namespace
-void DataLayout::clear() {
- LegalIntWidths.clear();
- IntAlignments.clear();
- FloatAlignments.clear();
- VectorAlignments.clear();
- Pointers.clear();
- delete static_cast<StructLayoutMap *>(LayoutMap);
- LayoutMap = nullptr;
-}
-
-DataLayout::~DataLayout() {
- clear();
-}
+DataLayout::~DataLayout() { delete static_cast<StructLayoutMap *>(LayoutMap); }
const StructLayout *DataLayout::getStructLayout(StructType *Ty) const {
if (!LayoutMap)
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 80b5408b61eda0..4738ec7639f57a 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -388,9 +388,7 @@ void Module::setModuleFlag(ModFlagBehavior Behavior, StringRef Key,
setModuleFlag(Behavior, Key, ConstantInt::get(Int32Ty, Val));
}
-void Module::setDataLayout(StringRef Desc) {
- DL.reset(Desc);
-}
+void Module::setDataLayout(StringRef Desc) { DL = DataLayout(Desc); }
void Module::setDataLayout(const DataLayout &Other) { DL = Other; }
>From becfa2f346fcca17503aaa511896657ddf26934e Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <s.barannikov at module.ru>
Date: Tue, 13 Aug 2024 14:44:38 +0300
Subject: [PATCH 2/2] Invalidate struct layout cache
---
llvm/lib/IR/DataLayout.cpp | 49 ++++++++++++++--------------
llvm/unittests/IR/DataLayoutTest.cpp | 17 ++++++++++
2 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 1411746a5f2f39..417e94b12995ed 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -117,6 +117,29 @@ unsigned StructLayout::getElementContainingOffset(uint64_t FixedOffset) const {
return SI - MemberOffsets.begin();
}
+namespace {
+
+class StructLayoutMap {
+ using LayoutInfoTy = DenseMap<StructType*, StructLayout*>;
+ LayoutInfoTy LayoutInfo;
+
+public:
+ ~StructLayoutMap() {
+ // Remove any layouts.
+ for (const auto &I : LayoutInfo) {
+ StructLayout *Value = I.second;
+ Value->~StructLayout();
+ free(Value);
+ }
+ }
+
+ StructLayout *&operator[](StructType *STy) {
+ return LayoutInfo[STy];
+ }
+};
+
+} // end anonymous namespace
+
//===----------------------------------------------------------------------===//
// LayoutAlignElem, LayoutAlign support
//===----------------------------------------------------------------------===//
@@ -214,7 +237,8 @@ DataLayout::DataLayout(StringRef LayoutString) {
}
DataLayout &DataLayout::operator=(const DataLayout &Other) {
- // Copy everything except for LayoutMap, which will be recomputed on demand.
+ delete static_cast<StructLayoutMap *>(LayoutMap);
+ LayoutMap = nullptr;
StringRepresentation = Other.StringRepresentation;
BigEndian = Other.BigEndian;
AllocaAddrSpace = Other.AllocaAddrSpace;
@@ -687,29 +711,6 @@ Align DataLayout::getIntegerAlignment(uint32_t BitWidth,
return abi_or_pref ? I->ABIAlign : I->PrefAlign;
}
-namespace {
-
-class StructLayoutMap {
- using LayoutInfoTy = DenseMap<StructType*, StructLayout*>;
- LayoutInfoTy LayoutInfo;
-
-public:
- ~StructLayoutMap() {
- // Remove any layouts.
- for (const auto &I : LayoutInfo) {
- StructLayout *Value = I.second;
- Value->~StructLayout();
- free(Value);
- }
- }
-
- StructLayout *&operator[](StructType *STy) {
- return LayoutInfo[STy];
- }
-};
-
-} // end anonymous namespace
-
DataLayout::~DataLayout() { delete static_cast<StructLayoutMap *>(LayoutMap); }
const StructLayout *DataLayout::getStructLayout(StructType *Ty) const {
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 4b711e87e1a9d2..649132b1bf9c06 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -19,6 +19,23 @@ using namespace llvm;
namespace {
+TEST(DataLayoutTest, CopyAssignmentInvalidatesStructLayout) {
+ DataLayout DL1 = cantFail(DataLayout::parse("p:32:32"));
+ DataLayout DL2 = cantFail(DataLayout::parse("p:64:64"));
+
+ LLVMContext Ctx;
+ StructType *Ty = StructType::get(PointerType::getUnqual(Ctx));
+
+ // Initialize struct layout caches.
+ EXPECT_EQ(DL1.getStructLayout(Ty)->getAlignment(), Align(4));
+ EXPECT_EQ(DL2.getStructLayout(Ty)->getAlignment(), Align(8));
+
+ // The copy should invalidate DL1's cache.
+ DL1 = DL2;
+ EXPECT_EQ(DL1.getStructLayout(Ty)->getAlignment(), Align(8));
+ EXPECT_EQ(DL2.getStructLayout(Ty)->getAlignment(), Align(8));
+}
+
TEST(DataLayoutTest, FunctionPtrAlign) {
EXPECT_EQ(MaybeAlign(0), DataLayout("").getFunctionPtrAlign());
EXPECT_EQ(MaybeAlign(1), DataLayout("Fi8").getFunctionPtrAlign());
More information about the llvm-commits
mailing list