https://github.com/andykaylor updated
https://github.com/llvm/llvm-project/pull/136036
>From fbe118b75d5edf8077b46d2a4fdbf94b6f2395c6 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akay...@nvidia.com>
Date: Wed, 16 Apr 2025 13:44:04 -0700
Subject: [PATCH 1/2] [CIR] Upstream support for record packing and padding
This change adds support for packing and padding record types in
ClangIR and introduces some infrastructure needed for this
computation.
Although union support has not been upstreamed yet, there was no
good way to report unions as NYI in the layout computation, so
the code added here includes layout computation for unions. Unions
will be added soon.
---
.../include/clang/CIR/Dialect/IR/CIRAttrs.td | 28 +++++
.../clang/CIR/Dialect/IR/CIRDataLayout.h | 33 +++++
.../include/clang/CIR/Dialect/IR/CIRTypes.td | 9 ++
clang/include/clang/CIR/MissingFeatures.h | 3 -
.../CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp | 104 ++++++++++++++--
clang/lib/CIR/Dialect/IR/CIRAttrs.cpp | 16 +++
clang/lib/CIR/Dialect/IR/CIRTypes.cpp | 113 +++++++++++++++++-
.../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 5 +-
clang/test/CIR/CodeGen/struct.c | 46 ++++++-
9 files changed, 332 insertions(+), 25 deletions(-)
create mode 100644 clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index 3680ded4afafe..792940857d60c 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -229,4 +229,32 @@ def ConstPtrAttr : CIR_Attr<"ConstPtr", "ptr",
[TypedAttrInterface]> {
}];
}
+//===----------------------------------------------------------------------===//
+// RecordLayoutAttr
+//===----------------------------------------------------------------------===//
+
+// Used to decouple layout information from the record type. RecordType's
+// uses this attribute to cache that information.
+
+def RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout"> {
+ let summary = "ABI specific information about a record layout";
+ let description = [{
+ Holds layout information often queried by !cir.record users
+ during lowering passes and optimizations.
+ }];
+
+ let parameters = (ins "unsigned":$size,
+ "unsigned":$alignment,
+ "bool":$padded,
+ "mlir::Type":$largest_member,
+ "mlir::ArrayAttr":$offsets);
+
+ let genVerifyDecl = 1;
+ let assemblyFormat = [{
+ `<`
+ struct($size, $alignment, $padded, $largest_member, $offsets)
+ `>`
+ }];
+}
+
#endif // LLVM_CLANG_CIR_DIALECT_IR_CIRATTRS_TD
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
b/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
new file mode 100644
index 0000000000000..62fc53f2362fb
--- /dev/null
+++ b/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Provides an LLVM-like API wrapper to DLTI and MLIR layout queries. This
+// makes it easier to port some of LLVM codegen layout logic to CIR.
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_CIR_DIALECT_IR_CIRDATALAYOUT_H
+#define CLANG_CIR_DIALECT_IR_CIRDATALAYOUT_H
+
+#include "mlir/IR/BuiltinOps.h"
+
+namespace cir {
+
+// TODO(cir): This might be replaced by a CIRDataLayout interface which can
+// provide the same functionalities.
+class CIRDataLayout {
+ // This is starting with the minimum functionality needed for code that is
+ // being upstreamed. Additional methods and members will be added as needed.
+public:
+ mlir::DataLayout layout;
+
+ /// Constructs a DataLayout the module's data layout attribute.
+ CIRDataLayout(mlir::ModuleOp modOp) : layout{modOp} {}
+};
+
+} // namespace cir
+
+#endif // CLANG_CIR_DIALECT_IR_CIRDATALAYOUT_H
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
index 23e20755dca95..3cec3701ab9b3 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -502,6 +502,15 @@ def CIR_RecordType : CIR_Type<"Record", "record",
void complete(llvm::ArrayRef<mlir::Type> members, bool packed,
bool isPadded);
+
+ // Utilities for lazily computing and cacheing data layout info.
+ // FIXME: currently opaque because there's a cycle if CIRTypes.types include
+ // from CIRAttrs.h. The implementation operates in terms of RecordLayoutAttr
+ // instead.
+ private:
+ mutable mlir::Attribute layoutInfo;
+ void computeSizeAndAlignment(const mlir::DataLayout &dataLayout) const;
+ public:
}];
let hasCustomAssemblyFormat = 1;
diff --git a/clang/include/clang/CIR/MissingFeatures.h
b/clang/include/clang/CIR/MissingFeatures.h
index 5ba55c53dfc4d..798fcb0900a91 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -102,13 +102,10 @@ struct MissingFeatures {
static bool shouldReverseUnaryCondOnBoolExpr() { return false; }
// RecordType
- static bool recordTypeLayoutInfo() { return false; }
static bool recursiveRecordLayout() { return false; }
static bool skippedLayout() { return false; }
static bool astRecordDeclAttr() { return false; }
static bool cxxSupport() { return false; }
- static bool packedRecords() { return false; }
- static bool recordPadding() { return false; }
static bool recordZeroInit() { return false; }
static bool zeroSizeRecordMembers() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 4e3adeaf50187..5e209b5b92503 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -19,6 +19,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/RecordLayout.h"
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
+#include "clang/CIR/Dialect/IR/CIRDataLayout.h"
#include "llvm/Support/Casting.h"
#include <memory>
@@ -57,8 +58,18 @@ struct CIRRecordLowering final {
CIRRecordLowering(CIRGenTypes &cirGenTypes, const RecordDecl *recordDecl,
bool isPacked);
+ /// Constructs a MemberInfo instance from an offset and mlir::Type.
+ MemberInfo makeStorageInfo(CharUnits offset, mlir::Type data) {
+ return MemberInfo(offset, MemberInfo::InfoKind::Field, data);
+ }
+
void lower();
+ /// Determines if we need a packed llvm struct.
+ void determinePacked();
+ /// Inserts padding everywhere it's needed.
+ void insertPadding();
+
void accumulateFields();
CharUnits bitsToCharUnits(uint64_t bitOffset) {
@@ -66,12 +77,33 @@ struct CIRRecordLowering final {
}
CharUnits getSize(mlir::Type Ty) {
- assert(!cir::MissingFeatures::recordTypeLayoutInfo());
- return CharUnits::One();
+ return CharUnits::fromQuantity(dataLayout.layout.getTypeSize(Ty));
}
CharUnits getAlignment(mlir::Type Ty) {
- assert(!cir::MissingFeatures::recordTypeLayoutInfo());
- return CharUnits::One();
+ return CharUnits::fromQuantity(dataLayout.layout.getTypeABIAlignment(Ty));
+ }
+
+ /// Wraps cir::IntType with some implicit arguments.
+ mlir::Type getUIntNType(uint64_t numBits) {
+ unsigned alignedBits = llvm::PowerOf2Ceil(numBits);
+ alignedBits = std::max(8u, alignedBits);
+ return cir::IntType::get(&cirGenTypes.getMLIRContext(), alignedBits,
+ /*isSigned=*/false);
+ }
+
+ mlir::Type getCharType() {
+ return cir::IntType::get(&cirGenTypes.getMLIRContext(),
+ astContext.getCharWidth(),
+ /*isSigned=*/false);
+ }
+
+ mlir::Type getByteArrayType(CharUnits numberOfChars) {
+ assert(!numberOfChars.isZero() && "Empty byte arrays aren't allowed.");
+ mlir::Type type = getCharType();
+ return numberOfChars == CharUnits::One()
+ ? type
+ : cir::ArrayType::get(type.getContext(), type,
+ numberOfChars.getQuantity());
}
mlir::Type getStorageType(const FieldDecl *fieldDecl) {
@@ -100,6 +132,7 @@ struct CIRRecordLowering final {
// Output fields, consumed by CIRGenTypes::computeRecordLayout
llvm::SmallVector<mlir::Type, 16> fieldTypes;
llvm::DenseMap<const FieldDecl *, unsigned> fields;
+ cir::CIRDataLayout dataLayout;
LLVM_PREFERRED_TYPE(bool)
unsigned zeroInitializable : 1;
@@ -121,6 +154,7 @@ CIRRecordLowering::CIRRecordLowering(CIRGenTypes
&cirGenTypes,
astContext(cirGenTypes.getASTContext()), recordDecl(recordDecl),
astRecordLayout(
cirGenTypes.getASTContext().getASTRecordLayout(recordDecl)),
+ dataLayout(cirGenTypes.getCGModule().getModule()),
zeroInitializable(true), packed(isPacked), padded(false) {}
void CIRRecordLowering::lower() {
@@ -138,18 +172,20 @@ void CIRRecordLowering::lower() {
assert(!cir::MissingFeatures::cxxSupport());
+ CharUnits size = astRecordLayout.getSize();
+
accumulateFields();
llvm::stable_sort(members);
// TODO: implement clipTailPadding once bitfields are implemented
assert(!cir::MissingFeatures::bitfields());
- // TODO: implemented packed records
- assert(!cir::MissingFeatures::packedRecords());
- // TODO: implement padding
- assert(!cir::MissingFeatures::recordPadding());
- // TODO: support zeroInit
assert(!cir::MissingFeatures::recordZeroInit());
+ members.push_back(makeStorageInfo(size, getUIntNType(8)));
+ determinePacked();
+ insertPadding();
+ members.pop_back();
+
fillOutputFields();
}
@@ -186,6 +222,56 @@ void CIRRecordLowering::accumulateFields() {
}
}
+void CIRRecordLowering::determinePacked() {
+ if (packed)
+ return;
+ CharUnits alignment = CharUnits::One();
+
+ // TODO(cir): handle non-virtual base types
+ assert(!cir::MissingFeatures::cxxSupport());
+
+ for (const MemberInfo &member : members) {
+ if (!member.data)
+ continue;
+ // If any member falls at an offset that it not a multiple of its
alignment,
+ // then the entire record must be packed.
+ if (member.offset % getAlignment(member.data))
+ packed = true;
+ alignment = std::max(alignment, getAlignment(member.data));
+ }
+ // If the size of the record (the capstone's offset) is not a multiple of the
+ // record's alignment, it must be packed.
+ if (members.back().offset % alignment)
+ packed = true;
+ // Update the alignment of the sentinel.
+ if (!packed)
+ members.back().data = getUIntNType(astContext.toBits(alignment));
+}
+
+void CIRRecordLowering::insertPadding() {
+ std::vector<std::pair<CharUnits, CharUnits>> padding;
+ CharUnits size = CharUnits::Zero();
+ for (const MemberInfo &member : members) {
+ if (!member.data)
+ continue;
+ CharUnits offset = member.offset;
+ assert(offset >= size);
+ // Insert padding if we need to.
+ if (offset !=
+ size.alignTo(packed ? CharUnits::One() : getAlignment(member.data)))
+ padding.push_back(std::make_pair(size, offset - size));
+ size = offset + getSize(member.data);
+ }
+ if (padding.empty())
+ return;
+ padded = true;
+ // Add the padding to the Members list and sort it.
+ for (const std::pair<CharUnits, CharUnits> &paddingPair : padding)
+ members.push_back(makeStorageInfo(paddingPair.first,
+ getByteArrayType(paddingPair.second)));
+ llvm::stable_sort(members);
+}
+
std::unique_ptr<CIRGenRecordLayout>
CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
CIRRecordLowering lowering(*this, rd, /*packed=*/false);
diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
index a8d9f6a0e6e9b..cc3baf1f348f9 100644
--- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
@@ -55,6 +55,22 @@ void CIRDialect::printAttribute(Attribute attr,
DialectAsmPrinter &os) const {
llvm_unreachable("unexpected CIR type kind");
}
+//===----------------------------------------------------------------------===//
+// RecordLayoutAttr definitions
+//===----------------------------------------------------------------------===//
+
+LogicalResult RecordLayoutAttr::verify(
+ ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, unsigned
size,
+ unsigned alignment, bool padded, mlir::Type largest_member,
+ mlir::ArrayAttr offsets) {
+ if (not std::all_of(offsets.begin(), offsets.end(), [](mlir::Attribute attr)
{
+ return mlir::isa<mlir::IntegerAttr>(attr);
+ })) {
+ return emitError() << "all index values must be integers";
+ }
+ return success();
+}
+
//===----------------------------------------------------------------------===//
// ConstPtrAttr definitions
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
index 160732d9c3610..527607c4c6d72 100644
--- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -177,6 +177,12 @@ void RecordType::print(mlir::AsmPrinter &printer) const {
// Type not yet printed: continue printing the entire record.
printer << ' ';
+ if (getPacked())
+ printer << "packed ";
+
+ if (getPadded())
+ printer << "padded ";
+
if (isIncomplete()) {
printer << "incomplete";
} else {
@@ -210,6 +216,10 @@ mlir::StringAttr RecordType::getName() const { return
getImpl()->name; }
bool RecordType::getIncomplete() const { return getImpl()->incomplete; }
+bool RecordType::getPacked() const { return getImpl()->packed; }
+
+bool RecordType::getPadded() const { return getImpl()->padded; }
+
cir::RecordType::RecordKind RecordType::getKind() const {
return getImpl()->kind;
}
@@ -225,17 +235,108 @@ void RecordType::complete(ArrayRef<Type> members, bool
packed, bool padded) {
//===----------------------------------------------------------------------===//
llvm::TypeSize
-RecordType::getTypeSizeInBits(const ::mlir::DataLayout &dataLayout,
- ::mlir::DataLayoutEntryListRef params) const {
- assert(!cir::MissingFeatures::recordTypeLayoutInfo());
- return llvm::TypeSize::getFixed(8);
+RecordType::getTypeSizeInBits(const mlir::DataLayout &dataLayout,
+ mlir::DataLayoutEntryListRef params) const {
+ if (!layoutInfo)
+ computeSizeAndAlignment(dataLayout);
+ return llvm::TypeSize::getFixed(
+ mlir::cast<cir::RecordLayoutAttr>(layoutInfo).getSize() * 8);
}
uint64_t
RecordType::getABIAlignment(const ::mlir::DataLayout &dataLayout,
::mlir::DataLayoutEntryListRef params) const {
- assert(!cir::MissingFeatures::recordTypeLayoutInfo());
- return 4;
+ if (!layoutInfo)
+ computeSizeAndAlignment(dataLayout);
+ return mlir::cast<cir::RecordLayoutAttr>(layoutInfo).getAlignment();
+}
+
+void RecordType::computeSizeAndAlignment(
+ const mlir::DataLayout &dataLayout) const {
+ assert(isComplete() && "Cannot get layout of incomplete records");
+ // Do not recompute.
+ if (layoutInfo)
+ return;
+
+ // This is a similar algorithm to LLVM's StructLayout.
+ unsigned recordSize = 0;
+ llvm::Align recordAlignment{1};
+ bool isPadded = false;
+ unsigned numElements = getNumElements();
+ ArrayRef<mlir::Type> members = getMembers();
+ mlir::Type largestMember;
+ unsigned largestMemberSize = 0;
+ llvm::SmallVector<mlir::Attribute, 4> memberOffsets;
+
+ bool dontCountLastElt = isUnion() && getPadded();
+ if (dontCountLastElt)
+ numElements--;
+
+ // Loop over each of the elements, placing them in memory.
+ memberOffsets.reserve(numElements);
+
+ // We can't use a range-based for loop here because we might be ignoring the
+ // last element.
+ for (unsigned i = 0, e = numElements; i != e; ++i) {
+ mlir::Type ty = members[i];
+
+ // If we're processing a union, we need to find the largest member.
+ if (isUnion() && (!largestMember ||
+ dataLayout.getTypeABIAlignment(ty) >
+ dataLayout.getTypeABIAlignment(largestMember) ||
+ (dataLayout.getTypeABIAlignment(ty) ==
+ dataLayout.getTypeABIAlignment(largestMember) &&
+ dataLayout.getTypeSize(ty) > largestMemberSize))) {
+ largestMember = ty;
+ largestMemberSize = dataLayout.getTypeSize(largestMember);
+ }
+
+ // This matches LLVM since it uses the ABI instead of preferred alignment.
+ const llvm::Align tyAlign =
+ llvm::Align(getPacked() ? 1 : dataLayout.getTypeABIAlignment(ty));
+
+ // Add padding if necessary to align the data element properly.
+ if (!llvm::isAligned(tyAlign, recordSize)) {
+ isPadded = true;
+ recordSize = llvm::alignTo(recordSize, tyAlign);
+ }
+
+ // Keep track of maximum alignment constraint.
+ recordAlignment = std::max(tyAlign, recordAlignment);
+
+ // Record size up to each element is the element offset.
+ memberOffsets.push_back(mlir::IntegerAttr::get(
+ mlir::IntegerType::get(getContext(), 32), isUnion() ? 0 : recordSize));
+
+ // Consume space for this data item
+ recordSize += dataLayout.getTypeSize(ty);
+ }
+
+ // For unions, the size and aligment is that of the largest element.
+ if (isUnion()) {
+ recordSize = largestMemberSize;
+ if (getPadded()) {
+ memberOffsets.push_back(mlir::IntegerAttr::get(
+ mlir::IntegerType::get(getContext(), 32), recordSize));
+ mlir::Type ty = members[numElements];
+ recordSize += dataLayout.getTypeSize(ty);
+ isPadded = true;
+ } else {
+ isPadded = false;
+ }
+ } else {
+ // Add padding to the end of the record so that it could be put in an array
+ // and all array elements would be aligned correctly.
+ if (!llvm::isAligned(recordAlignment, recordSize)) {
+ isPadded = true;
+ recordSize = llvm::alignTo(recordSize, recordAlignment);
+ }
+ }
+
+ auto offsets = mlir::ArrayAttr::get(getContext(), memberOffsets);
+ layoutInfo = cir::RecordLayoutAttr::get(getContext(), recordSize,
+ recordAlignment.value(), isPadded,
+ largestMember, offsets);
}
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index cb318c88c09c5..8c4a67258df3f 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1356,12 +1356,11 @@ static void
prepareTypeConverter(mlir::LLVMTypeConverter &converter,
if (type.getName()) {
llvmStruct = mlir::LLVM::LLVMStructType::getIdentified(
type.getContext(), type.getPrefixedName());
- assert(!cir::MissingFeatures::packedRecords());
- if (llvmStruct.setBody(llvmMembers, /*isPacked=*/true).failed())
+ if (llvmStruct.setBody(llvmMembers, type.getPacked()).failed())
llvm_unreachable("Failed to set body of record");
} else { // Record has no name: lower as literal record.
llvmStruct = mlir::LLVM::LLVMStructType::getLiteral(
- type.getContext(), llvmMembers, /*isPacked=*/true);
+ type.getContext(), llvmMembers, type.getPacked());
}
return llvmStruct;
diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c
index e1b01e6a8e86c..55f316f27eeb1 100644
--- a/clang/test/CIR/CodeGen/struct.c
+++ b/clang/test/CIR/CodeGen/struct.c
@@ -5,9 +5,19 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+// For LLVM IR checks, the structs are defined before the variables, so these
+// checks are at the top.
+// LLVM: %struct.CompleteS = type { i32, i8 }
+// LLVM: %struct.PackedS = type <{ i32, i8 }>
+// LLVM: %struct.PackedAndPaddedS = type <{ i32, i8, i8 }>
+// OGCG: %struct.CompleteS = type { i32, i8 }
+// OGCG: %struct.PackedS = type <{ i32, i8 }>
+// OGCG: %struct.PackedAndPaddedS = type <{ i32, i8, i8 }>
+
struct IncompleteS *p;
-// CIR: cir.global external @p = #cir.ptr<null> : !cir.ptr<!cir.record<struct
"IncompleteS" incomplete>>
+// CIR: cir.global external @p = #cir.ptr<null> :
!cir.ptr<!cir.record<struct
+// CIR-SAME: "IncompleteS" incomplete>>
// LLVM: @p = dso_local global ptr null
// OGCG: @p = global ptr null, align 8
@@ -16,17 +26,44 @@ struct CompleteS {
char b;
} cs;
-// CIR: cir.global external @cs = #cir.zero : !cir.record<struct
"CompleteS" {!s32i, !s8i}>
+// CIR: cir.global external @cs = #cir.zero : !cir.record<struct
+// CIR-SAME: "CompleteS" {!s32i, !s8i}>
// LLVM: @cs = dso_local global %struct.CompleteS zeroinitializer
// OGCG: @cs = global %struct.CompleteS zeroinitializer, align 4
+#pragma pack(push)
+#pragma pack(1)
+
+struct PackedS {
+ int a0;
+ char a1;
+} ps;
+
+// CIR: cir.global external @ps = #cir.zero : !cir.record<struct
"PackedS"
+// CIR-SAME: packed {!s32i, !s8i}>
+// LLVM: @ps = dso_local global %struct.PackedS zeroinitializer
+// OGCG: @ps = global %struct.PackedS zeroinitializer, align 1
+
+struct PackedAndPaddedS {
+ int b0;
+ char b1;
+} __attribute__((aligned(2))) pps;
+
+// CIR: cir.global external @pps = #cir.zero : !cir.record<struct
+// CIR-SAME: "PackedAndPaddedS" packed padded {!s32i, !s8i, !u8i}>
+// LLVM: @pps = dso_local global %struct.PackedAndPaddedS zeroinitializer
+// OGCG: @pps = global %struct.PackedAndPaddedS zeroinitializer, align 2
+
+#pragma pack(pop)
+
void f(void) {
struct IncompleteS *p;
}
// CIR: cir.func @f()
// CIR-NEXT: cir.alloca !cir.ptr<!cir.record<struct "IncompleteS"
incomplete>>,
-// CIR-SAME: !cir.ptr<!cir.ptr<!cir.record<struct "IncompleteS"
incomplete>>>, ["p"]
+// CIR-SAME: !cir.ptr<!cir.ptr<!cir.record<struct
+// CIR-SAME: "IncompleteS" incomplete>>>, ["p"]
// CIR-NEXT: cir.return
// LLVM: define void @f()
@@ -44,7 +81,8 @@ void f2(void) {
// CIR: cir.func @f2()
// CIR-NEXT: cir.alloca !cir.record<struct "CompleteS" {!s32i, !s8i}>,
-// CIR-SAME: !cir.ptr<!cir.record<struct "CompleteS" {!s32i, !s8i}>>,
["s"]
+// CIR-SAME: !cir.ptr<!cir.record<struct "CompleteS" {!s32i, !s8i}>>,
+// CIR-SAME: ["s"] {alignment = 4 : i64}
// CIR-NEXT: cir.return
// LLVM: define void @f2()
>From 3a24d33ae333082dde316b5134f118e584319be3 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akay...@nvidia.com>
Date: Thu, 17 Apr 2025 15:18:40 -0700
Subject: [PATCH 2/2] Remove RecordLayoutAttr, stop falsely caching layout info
---
.../include/clang/CIR/Dialect/IR/CIRAttrs.td | 28 ----
.../include/clang/CIR/Dialect/IR/CIRTypes.td | 8 +-
clang/lib/CIR/Dialect/IR/CIRAttrs.cpp | 16 --
clang/lib/CIR/Dialect/IR/CIRTypes.cpp | 140 ++++++++----------
4 files changed, 62 insertions(+), 130 deletions(-)
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index 792940857d60c..3680ded4afafe 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -229,32 +229,4 @@ def ConstPtrAttr : CIR_Attr<"ConstPtr", "ptr",
[TypedAttrInterface]> {
}];
}
-//===----------------------------------------------------------------------===//
-// RecordLayoutAttr
-//===----------------------------------------------------------------------===//
-
-// Used to decouple layout information from the record type. RecordType's
-// uses this attribute to cache that information.
-
-def RecordLayoutAttr : CIR_Attr<"RecordLayout", "record_layout"> {
- let summary = "ABI specific information about a record layout";
- let description = [{
- Holds layout information often queried by !cir.record users
- during lowering passes and optimizations.
- }];
-
- let parameters = (ins "unsigned":$size,
- "unsigned":$alignment,
- "bool":$padded,
- "mlir::Type":$largest_member,
- "mlir::ArrayAttr":$offsets);
-
- let genVerifyDecl = 1;
- let assemblyFormat = [{
- `<`
- struct($size, $alignment, $padded, $largest_member, $offsets)
- `>`
- }];
-}
-
#endif // LLVM_CLANG_CIR_DIALECT_IR_CIRATTRS_TD
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
index 3cec3701ab9b3..b028bc7db4e59 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -503,13 +503,9 @@ def CIR_RecordType : CIR_Type<"Record", "record",
void complete(llvm::ArrayRef<mlir::Type> members, bool packed,
bool isPadded);
- // Utilities for lazily computing and cacheing data layout info.
- // FIXME: currently opaque because there's a cycle if CIRTypes.types include
- // from CIRAttrs.h. The implementation operates in terms of RecordLayoutAttr
- // instead.
private:
- mutable mlir::Attribute layoutInfo;
- void computeSizeAndAlignment(const mlir::DataLayout &dataLayout) const;
+ unsigned computeStructSize(const mlir::DataLayout &dataLayout) const;
+ uint64_t computeStructAlignment(const mlir::DataLayout &dataLayout) const;
public:
}];
diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
index cc3baf1f348f9..a8d9f6a0e6e9b 100644
--- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
@@ -55,22 +55,6 @@ void CIRDialect::printAttribute(Attribute attr,
DialectAsmPrinter &os) const {
llvm_unreachable("unexpected CIR type kind");
}
-//===----------------------------------------------------------------------===//
-// RecordLayoutAttr definitions
-//===----------------------------------------------------------------------===//
-
-LogicalResult RecordLayoutAttr::verify(
- ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, unsigned
size,
- unsigned alignment, bool padded, mlir::Type largest_member,
- mlir::ArrayAttr offsets) {
- if (not std::all_of(offsets.begin(), offsets.end(), [](mlir::Attribute attr)
{
- return mlir::isa<mlir::IntegerAttr>(attr);
- })) {
- return emitError() << "all index values must be integers";
- }
- return success();
-}
-
//===----------------------------------------------------------------------===//
// ConstPtrAttr definitions
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
index 527607c4c6d72..0bf8a8beee565 100644
--- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -237,106 +237,86 @@ void RecordType::complete(ArrayRef<Type> members, bool
packed, bool padded) {
llvm::TypeSize
RecordType::getTypeSizeInBits(const mlir::DataLayout &dataLayout,
mlir::DataLayoutEntryListRef params) const {
- if (!layoutInfo)
- computeSizeAndAlignment(dataLayout);
- return llvm::TypeSize::getFixed(
- mlir::cast<cir::RecordLayoutAttr>(layoutInfo).getSize() * 8);
+ if (isUnion()) {
+ // TODO(CIR): Implement union layout.
+ return llvm::TypeSize::getFixed(8);
+ }
+
+ unsigned recordSize = computeStructSize(dataLayout);
+ return llvm::TypeSize::getFixed(recordSize * 8);
}
uint64_t
RecordType::getABIAlignment(const ::mlir::DataLayout &dataLayout,
::mlir::DataLayoutEntryListRef params) const {
- if (!layoutInfo)
- computeSizeAndAlignment(dataLayout);
- return mlir::cast<cir::RecordLayoutAttr>(layoutInfo).getAlignment();
+ if (isUnion()) {
+ // TODO(CIR): Implement union layout.
+ return 8;
+ }
+
+ // Packed structures always have an ABI alignment of 1.
+ if (getPacked())
+ return 1;
+ return computeStructAlignment(dataLayout);
}
-void RecordType::computeSizeAndAlignment(
- const mlir::DataLayout &dataLayout) const {
+unsigned
+RecordType::computeStructSize(const mlir::DataLayout &dataLayout) const {
assert(isComplete() && "Cannot get layout of incomplete records");
- // Do not recompute.
- if (layoutInfo)
- return;
// This is a similar algorithm to LLVM's StructLayout.
unsigned recordSize = 0;
- llvm::Align recordAlignment{1};
- bool isPadded = false;
- unsigned numElements = getNumElements();
- ArrayRef<mlir::Type> members = getMembers();
- mlir::Type largestMember;
- unsigned largestMemberSize = 0;
- llvm::SmallVector<mlir::Attribute, 4> memberOffsets;
-
- bool dontCountLastElt = isUnion() && getPadded();
- if (dontCountLastElt)
- numElements--;
-
- // Loop over each of the elements, placing them in memory.
- memberOffsets.reserve(numElements);
+ uint64_t recordAlignment = 1;
// We can't use a range-based for loop here because we might be ignoring the
// last element.
- for (unsigned i = 0, e = numElements; i != e; ++i) {
- mlir::Type ty = members[i];
-
- // If we're processing a union, we need to find the largest member.
- if (isUnion() && (!largestMember ||
- dataLayout.getTypeABIAlignment(ty) >
- dataLayout.getTypeABIAlignment(largestMember) ||
- (dataLayout.getTypeABIAlignment(ty) ==
- dataLayout.getTypeABIAlignment(largestMember) &&
- dataLayout.getTypeSize(ty) > largestMemberSize))) {
- largestMember = ty;
- largestMemberSize = dataLayout.getTypeSize(largestMember);
- }
-
- // This matches LLVM since it uses the ABI instead of preferred alignment.
- const llvm::Align tyAlign =
- llvm::Align(getPacked() ? 1 : dataLayout.getTypeABIAlignment(ty));
-
- // Add padding if necessary to align the data element properly.
- if (!llvm::isAligned(tyAlign, recordSize)) {
- isPadded = true;
- recordSize = llvm::alignTo(recordSize, tyAlign);
- }
+ for (mlir::Type ty : getMembers()) {
+ // This assumes that we're calculating size based on the ABI alignment, not
+ // the preferred alignment for each type.
+ const uint64_t tyAlign =
+ (getPacked() ? 1 : dataLayout.getTypeABIAlignment(ty));
+
+ // This should be aligned because the padding is inserted when we build
+ // the record.
+ assert(llvm::isAligned(llvm::Align(tyAlign), recordSize));
+
+ // 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.
+ recordSize = llvm::alignTo(recordSize, tyAlign);
+ recordSize += dataLayout.getTypeSize(ty);
- // Keep track of maximum alignment constraint.
+ // The alignment requirement of a struct is equal to the strictest
alignment
+ // requirement of its elements.
recordAlignment = std::max(tyAlign, recordAlignment);
+ }
- // Record size up to each element is the element offset.
- memberOffsets.push_back(mlir::IntegerAttr::get(
- mlir::IntegerType::get(getContext(), 32), isUnion() ? 0 : recordSize));
+ // Add padding to the end of the record so that it could be put in an array
+ // and all array elements would be aligned correctly.
+ assert(llvm::isAligned(llvm::Align(recordAlignment), recordSize));
+ // if (!llvm::isAligned(recordAlignment, recordSize))
+ // recordSize = llvm::alignTo(recordSize, recordAlignment);
- // Consume space for this data item
- recordSize += dataLayout.getTypeSize(ty);
- }
+ // At the end, add padding to the struct to satisfy its own alignment
+ // requirement. Otherwise structs inside of arrays would be misaligned.
+ recordSize = llvm::alignTo(recordSize, recordAlignment);
+ return recordSize;
+}
- // For unions, the size and aligment is that of the largest element.
- if (isUnion()) {
- recordSize = largestMemberSize;
- if (getPadded()) {
- memberOffsets.push_back(mlir::IntegerAttr::get(
- mlir::IntegerType::get(getContext(), 32), recordSize));
- mlir::Type ty = members[numElements];
- recordSize += dataLayout.getTypeSize(ty);
- isPadded = true;
- } else {
- isPadded = false;
- }
- } else {
- // Add padding to the end of the record so that it could be put in an array
- // and all array elements would be aligned correctly.
- if (!llvm::isAligned(recordAlignment, recordSize)) {
- isPadded = true;
- recordSize = llvm::alignTo(recordSize, recordAlignment);
- }
- }
+// We also compute the alignment as part of computeStructSize, but this is more
+// efficient. Ideally, we'd like to compute both at once and cache the result,
+// but that's implemented yet.
+// TODO(CIR): Implement a way to cache the result.
+uint64_t
+RecordType::computeStructAlignment(const mlir::DataLayout &dataLayout) const {
+ assert(isComplete() && "Cannot get layout of incomplete records");
+
+ // This is a similar algorithm to LLVM's StructLayout.
+ uint64_t recordAlignment = 1;
+ for (mlir::Type ty : getMembers())
+ recordAlignment =
+ std::max(dataLayout.getTypeABIAlignment(ty), recordAlignment);
- auto offsets = mlir::ArrayAttr::get(getContext(), memberOffsets);
- layoutInfo = cir::RecordLayoutAttr::get(getContext(), recordSize,
- recordAlignment.value(), isPadded,
- largestMember, offsets);
+ return recordAlignment;
}
//===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
