[DataLayout] Remove clear and reset methods (NFC)

[s-barannikov]

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.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Sergei Barannikov (s-barannikov)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/102993.diff

3 Files Affected:

• (modified) llvm/include/llvm/IR/DataLayout.h (+3-10)
• (modified) llvm/lib/IR/DataLayout.cpp (+7-23)
• (modified) llvm/lib/IR/Module.cpp (+1-1)

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..bf8ebc5b8c04bf 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,18 +710,8 @@ 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();
+  delete static_cast<StructLayoutMap *>(LayoutMap);
 }
 
 const StructLayout *DataLayout::getStructLayout(StructType *Ty) const {
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 80b5408b61eda0..3da05b79c874eb 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -389,7 +389,7 @@ void Module::setModuleFlag(ModFlagBehavior Behavior, StringRef Key,
 }
 
 void Module::setDataLayout(StringRef Desc) {
-  DL.reset(Desc);
+  DL = DataLayout(Desc);
 }
 
 void Module::setDataLayout(const DataLayout &Other) { DL = Other; }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

Shouldn't we explicitly clear it then?

[s-barannikov]

Yes, of course.
For some reason I assumed that the operator should work the same way as a copy constructor.
Thanks for catching.

[s-barannikov]

Should be fixed. I had to move class declaration to make it visible to delete.

[nikic]

LGTM

[maleadt]

This inadvertently caused some issues with the Julia JIT, where we used DataLayout::reset to replace a TargetMachine's data layout with one that marks certain address spaces as non-integral. I realize that isn't kosher, so what would be the "proper" way of creating a TargetMachine with a custom DataLayout? createTargetMachine doesn't have any customization.

FWIW, ignoring the mismatch between the TM's data layout and the one we use on modules causes issues down the line, e.g., when MachineFunction asserts that both match:

llvm-project/llvm/lib/CodeGen/MachineFunction.cpp

Lines 243 to 245 in 22576e2

	assert(Target.isCompatibleDataLayout(getDataLayout()) &&
	"Can't create a MachineFunction using a Module with a "
	"Target-incompatible DataLayout attached\n");

[s-barannikov]

we used DataLayout::reset to replace a TargetMachine's data layout with one that marks certain address spaces as non-integral

TargetMachine's DL member is const and createDataLayout() returns a const copy of it. How did you manage to call reset() on a copy and get it working? If you modified TargetMachine.h, I guess you can also replace reset() with copy assignment?

[maleadt]

createDataLayout() returns a const copy of it.

You're right, I misdiagnosed the issue (which did appear after upgrading to LLVM 20, so this change seemed likely).