[Mlir-commits] [mlir] [mlir][LLVM] Use element allocation sizes when computing struct size (PR #221742)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Risan Gnanasegaram (risan-g)

<details>
<summary>Changes</summary>

`LLVMStructType::getTypeSizeInBits` currently advances by each element's
store size. With the data layout from #<!-- -->221665, this reports 11 bytes for
`!llvm.struct<packed (f80, i8)>`, while LLVM uses the 16-byte allocation
size of `f80` and reports a 17-byte struct.

Round each element's size up to its ABI alignment before adding it to the
struct size, matching LLVM's `getTypeAllocSize` behavior. Packed structs
continue to suppress alignment gaps before elements, while each element
still consumes its allocation size.

Add regression coverage for packed, unpacked, nested, and array layouts.

Fixes #<!-- -->221665.

Testing: 99 relevant MLIR tests passed; 3 x86-only tests were unsupported
on the ARM host.

Assisted by: OpenAI Codex

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


2 Files Affected:

- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp (+4-3) 
- (modified) mlir/test/Dialect/LLVMIR/layout.mlir (+53) 


``````````diff
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index ba2b8de4d7d826..92df593860c01a 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -560,10 +560,11 @@ LLVMStructType::getTypeSizeInBits(const DataLayout &dataLayout,
   for (Type element : getBody()) {
     uint64_t elementAlignment =
         isPacked() ? 1 : dataLayout.getTypeABIAlignment(element);
-    // Add padding to the struct size to align it to the abi alignment of the
-    // element type before than adding the size of the element.
+    // Add padding to align the element unless the struct is packed.
     structSize = llvm::alignTo(structSize, elementAlignment);
-    structSize += dataLayout.getTypeSize(element);
+    // Elements occupy their allocation size, even in packed structs.
+    structSize += llvm::alignTo(dataLayout.getTypeSize(element),
+                                dataLayout.getTypeABIAlignment(element));
 
     // The alignment requirement of a struct is equal to the strictest alignment
     // requirement of its elements.
diff --git a/mlir/test/Dialect/LLVMIR/layout.mlir b/mlir/test/Dialect/LLVMIR/layout.mlir
index c650ca3bc1a88a..2bdad3539a4f05 100644
--- a/mlir/test/Dialect/LLVMIR/layout.mlir
+++ b/mlir/test/Dialect/LLVMIR/layout.mlir
@@ -221,6 +221,59 @@ module {
 
 // -----
 
+module attributes { dlti.dl_spec = #dlti.dl_spec<
+  i8 = dense<8> : vector<2xi64>,
+  i24 = dense<[32, 64]> : vector<2xi64>,
+  f80 = dense<128> : vector<2xi64>
+>} {
+  // CHECK-LABEL: @struct_element_allocation_size
+  func.func @struct_element_allocation_size() {
+    // A packed element still occupies its allocation size, including padding.
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 128
+    // CHECK-SAME: size = 16
+    "test.data_layout_query"() : () -> !llvm.struct<packed (f80)>
+
+    // The i8 follows the 16-byte allocation of f80, not its 10-byte store size.
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 136
+    // CHECK-SAME: size = 17
+    "test.data_layout_query"() : () -> !llvm.struct<packed (f80, i8)>
+
+    // Packing suppresses alignment gaps before elements and at the struct end.
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 144
+    // CHECK-SAME: size = 18
+    "test.data_layout_query"() : () -> !llvm.struct<packed (i8, f80, i8)>
+
+    // Unpacked structs also use element allocation sizes.
+    // CHECK: alignment = 16
+    // CHECK-SAME: bitsize = 256
+    // CHECK-SAME: size = 32
+    "test.data_layout_query"() : () -> !llvm.struct<(f80, i8)>
+
+    // Integer allocation sizes use ABI alignment, not preferred alignment.
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 40
+    // CHECK-SAME: size = 5
+    "test.data_layout_query"() : () -> !llvm.struct<packed (i24, i8)>
+
+    // The corrected size propagates through nested structs and arrays.
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 144
+    // CHECK-SAME: size = 18
+    "test.data_layout_query"() : () -> !llvm.struct<packed (struct<packed (f80, i8)>, i8)>
+
+    // CHECK: alignment = 1
+    // CHECK-SAME: bitsize = 272
+    // CHECK-SAME: size = 34
+    "test.data_layout_query"() : () -> !llvm.array<2 x struct<packed (f80, i8)>>
+    return
+  }
+}
+
+// -----
+
 module attributes { dlti.dl_spec = #dlti.dl_spec<
   #dlti.dl_entry<!llvm.struct<()>, dense<[32, 32]> : vector<2xi64>>
 >} {

``````````

</details>


https://github.com/llvm/llvm-project/pull/221742