[llvm] b1aa0b0 - [DataLayout] Remove `clear` and `reset` methods (NFC) (#102993)

via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 13 10:10:39 PDT 2024


Author: Sergei Barannikov
Date: 2024-08-13T20:10:35+03:00
New Revision: b1aa0b0b88a0bca2553f8c6d6c52ec3b7205064b

URL: https://github.com/llvm/llvm-project/commit/b1aa0b0b88a0bca2553f8c6d6c52ec3b7205064b
DIFF: https://github.com/llvm/llvm-project/commit/b1aa0b0b88a0bca2553f8c6d6c52ec3b7205064b.diff

LOG: [DataLayout] Remove `clear` and `reset` methods (NFC) (#102993)

`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.

Pull Reuquest: https://github.com/llvm/llvm-project/pull/102993

Added: 
    

Modified: 
    llvm/include/llvm/IR/DataLayout.h
    llvm/lib/IR/DataLayout.cpp
    llvm/lib/IR/Module.cpp
    llvm/unittests/IR/DataLayoutTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 9619560f4baed..f4b4b730bee2a 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 1efa61e92d2bc..ae0ff9c5ffe91 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -117,6 +117,27 @@ 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
 //===----------------------------------------------------------------------===//
@@ -191,36 +212,31 @@ 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();
+  delete static_cast<StructLayoutMap *>(LayoutMap);
+  LayoutMap = nullptr;
   StringRepresentation = Other.StringRepresentation;
   BigEndian = Other.BigEndian;
   AllocaAddrSpace = Other.AllocaAddrSpace;
@@ -693,42 +709,7 @@ 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
-
-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 80b5408b61eda..4738ec7639f57 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; }
 

diff  --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 4b711e87e1a9d..113bb578f6bc3 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -19,6 +19,27 @@ 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)->getSizeInBits(), 32U);
+  EXPECT_EQ(DL1.getStructLayout(Ty)->getAlignment(), Align(4));
+  EXPECT_EQ(DL2.getStructLayout(Ty)->getSizeInBits(), 64U);
+  EXPECT_EQ(DL2.getStructLayout(Ty)->getAlignment(), Align(8));
+
+  // The copy should invalidate DL1's cache.
+  DL1 = DL2;
+  EXPECT_EQ(DL1.getStructLayout(Ty)->getSizeInBits(), 64U);
+  EXPECT_EQ(DL1.getStructLayout(Ty)->getAlignment(), Align(8));
+  EXPECT_EQ(DL2.getStructLayout(Ty)->getSizeInBits(), 64U);
+  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