Skip to content

Commit 2267e5d

Browse files
authored
[CIR][CIRGen] Add padding to unions (#1289)
This PR adds padding for union type, which is necessary in some cases (e.g. proper offset computation for an element of an array). The previous discussion is here #1281 The idea is to add a notion about padding in the `StructType` in the same fashion as it's done for packed structures - as a bool argument in the constructor. Now we can compute the proper union type size as a size of the largest element + size of padding type. There are some downsides though - I had to add this `padded` word in many places. So take a look please! There are many tests fixed and one new - `union-padding`
1 parent d329c96 commit 2267e5d

24 files changed

+197
-113
lines changed

clang/include/clang/CIR/Dialect/IR/CIRTypes.h

+11-9
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ class StructType
8686
/// Create a identified and complete struct type.
8787
static StructType get(mlir::MLIRContext *context,
8888
llvm::ArrayRef<mlir::Type> members,
89-
mlir::StringAttr name, bool packed, RecordKind kind,
90-
ASTRecordDeclInterface ast = {});
89+
mlir::StringAttr name, bool packed, bool padded,
90+
RecordKind kind, ASTRecordDeclInterface ast = {});
9191
static StructType
9292
getChecked(llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
9393
mlir::MLIRContext *context, llvm::ArrayRef<mlir::Type> members,
94-
mlir::StringAttr name, bool packed, RecordKind kind,
94+
mlir::StringAttr name, bool packed, bool padded, RecordKind kind,
9595
ASTRecordDeclInterface ast = {});
9696

9797
/// Create a identified and incomplete struct type.
@@ -105,18 +105,20 @@ class StructType
105105
/// Create a anonymous struct type (always complete).
106106
static StructType get(mlir::MLIRContext *context,
107107
llvm::ArrayRef<mlir::Type> members, bool packed,
108-
RecordKind kind, ASTRecordDeclInterface ast = {});
108+
bool padded, RecordKind kind,
109+
ASTRecordDeclInterface ast = {});
109110
static StructType
110111
getChecked(llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
111112
mlir::MLIRContext *context, llvm::ArrayRef<mlir::Type> members,
112-
bool packed, RecordKind kind, ASTRecordDeclInterface ast = {});
113+
bool packed, bool padded, RecordKind kind,
114+
ASTRecordDeclInterface ast = {});
113115

114116
/// Validate the struct about to be constructed.
115117
static llvm::LogicalResult
116118
verifyInvariants(llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
117119
llvm::ArrayRef<mlir::Type> members, mlir::StringAttr name,
118-
bool incomplete, bool packed, StructType::RecordKind kind,
119-
ASTRecordDeclInterface ast);
120+
bool incomplete, bool packed, bool padded,
121+
StructType::RecordKind kind, ASTRecordDeclInterface ast);
120122

121123
// Parse/print methods.
122124
static constexpr llvm::StringLiteral getMnemonic() { return {"struct"}; }
@@ -130,6 +132,7 @@ class StructType
130132
StructType::RecordKind getKind() const;
131133
bool getIncomplete() const;
132134
bool getPacked() const;
135+
bool getPadded() const;
133136
void dropAst();
134137

135138
// Predicates
@@ -157,7 +160,7 @@ class StructType
157160
}
158161

159162
/// Complete the struct type by mutating its members and attributes.
160-
void complete(llvm::ArrayRef<mlir::Type> members, bool packed,
163+
void complete(llvm::ArrayRef<mlir::Type> members, bool packed, bool isPadded,
161164
ASTRecordDeclInterface ast = {});
162165

163166
/// DataLayoutTypeInterface methods.
@@ -178,7 +181,6 @@ class StructType
178181
// from CIRAttrs.h. The implementation operates in terms of StructLayoutAttr
179182
// instead.
180183
mutable mlir::Attribute layoutInfo;
181-
bool isPadded(const mlir::DataLayout &dataLayout) const;
182184
void computeSizeAndAlignment(const mlir::DataLayout &dataLayout) const;
183185
};
184186

clang/include/clang/CIR/Dialect/IR/CIRTypesDetails.h

+19-15
Original file line numberDiff line numberDiff line change
@@ -32,53 +32,55 @@ struct StructTypeStorage : public mlir::TypeStorage {
3232
mlir::StringAttr name;
3333
bool incomplete;
3434
bool packed;
35+
bool padded;
3536
StructType::RecordKind kind;
3637
ASTRecordDeclInterface ast;
3738

3839
KeyTy(llvm::ArrayRef<mlir::Type> members, mlir::StringAttr name,
39-
bool incomplete, bool packed, StructType::RecordKind kind,
40-
ASTRecordDeclInterface ast)
40+
bool incomplete, bool packed, bool padded,
41+
StructType::RecordKind kind, ASTRecordDeclInterface ast)
4142
: members(members), name(name), incomplete(incomplete), packed(packed),
42-
kind(kind), ast(ast) {}
43+
padded(padded), kind(kind), ast(ast) {}
4344
};
4445

4546
llvm::ArrayRef<mlir::Type> members;
4647
mlir::StringAttr name;
4748
bool incomplete;
4849
bool packed;
50+
bool padded;
4951
StructType::RecordKind kind;
5052
ASTRecordDeclInterface ast;
5153

5254
StructTypeStorage(llvm::ArrayRef<mlir::Type> members, mlir::StringAttr name,
53-
bool incomplete, bool packed, StructType::RecordKind kind,
54-
ASTRecordDeclInterface ast)
55+
bool incomplete, bool packed, bool padded,
56+
StructType::RecordKind kind, ASTRecordDeclInterface ast)
5557
: members(members), name(name), incomplete(incomplete), packed(packed),
56-
kind(kind), ast(ast) {}
58+
padded(padded), kind(kind), ast(ast) {}
5759

5860
KeyTy getAsKey() const {
59-
return KeyTy(members, name, incomplete, packed, kind, ast);
61+
return KeyTy(members, name, incomplete, packed, padded, kind, ast);
6062
}
6163

6264
bool operator==(const KeyTy &key) const {
6365
if (name)
6466
return (name == key.name) && (kind == key.kind);
6567
return (members == key.members) && (name == key.name) &&
6668
(incomplete == key.incomplete) && (packed == key.packed) &&
67-
(kind == key.kind) && (ast == key.ast);
69+
(padded == key.padded) && (kind == key.kind) && (ast == key.ast);
6870
}
6971

7072
static llvm::hash_code hashKey(const KeyTy &key) {
7173
if (key.name)
7274
return llvm::hash_combine(key.name, key.kind);
73-
return llvm::hash_combine(key.members, key.incomplete, key.packed, key.kind,
74-
key.ast);
75+
return llvm::hash_combine(key.members, key.incomplete, key.packed,
76+
key.padded, key.kind, key.ast);
7577
}
7678

7779
static StructTypeStorage *construct(mlir::TypeStorageAllocator &allocator,
7880
const KeyTy &key) {
79-
return new (allocator.allocate<StructTypeStorage>())
80-
StructTypeStorage(allocator.copyInto(key.members), key.name,
81-
key.incomplete, key.packed, key.kind, key.ast);
81+
return new (allocator.allocate<StructTypeStorage>()) StructTypeStorage(
82+
allocator.copyInto(key.members), key.name, key.incomplete, key.packed,
83+
key.padded, key.kind, key.ast);
8284
}
8385

8486
/// Mutates the members and attributes an identified struct.
@@ -89,20 +91,22 @@ struct StructTypeStorage : public mlir::TypeStorage {
8991
/// change the struct.
9092
llvm::LogicalResult mutate(mlir::TypeStorageAllocator &allocator,
9193
llvm::ArrayRef<mlir::Type> members, bool packed,
92-
ASTRecordDeclInterface ast) {
94+
bool padded, ASTRecordDeclInterface ast) {
9395
// Anonymous structs cannot mutate.
9496
if (!name)
9597
return llvm::failure();
9698

9799
// Mutation of complete structs are allowed if they change nothing.
98100
if (!incomplete)
99101
return mlir::success((this->members == members) &&
100-
(this->packed == packed) && (this->ast == ast));
102+
(this->packed == packed) &&
103+
(this->padded == padded) && (this->ast == ast));
101104

102105
// Mutate incomplete struct.
103106
this->members = allocator.copyInto(members);
104107
this->packed = packed;
105108
this->ast = ast;
109+
this->padded = padded;
106110

107111
incomplete = false;
108112
return llvm::success();

clang/lib/CIR/CodeGen/CIRAsm.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,8 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &S) {
619619
ResultType = ResultRegTypes[0];
620620
else if (ResultRegTypes.size() > 1) {
621621
auto sname = builder.getUniqueAnonRecordName();
622-
ResultType =
623-
builder.getCompleteStructTy(ResultRegTypes, sname, false, nullptr);
622+
ResultType = builder.getCompleteStructTy(ResultRegTypes, sname, false,
623+
false, nullptr);
624624
}
625625

626626
bool HasSideEffect = S.isVolatile() || S.getNumOutputs() == 0;

clang/lib/CIR/CodeGen/CIRGenBuilder.h

+15-12
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
178178

179179
mlir::Attribute getConstStructOrZeroAttr(mlir::ArrayAttr arrayAttr,
180180
bool packed = false,
181+
bool padded = false,
181182
mlir::Type type = {}) {
182183
llvm::SmallVector<mlir::Type, 8> members;
183184
auto structTy = mlir::dyn_cast<cir::StructType>(type);
@@ -193,9 +194,9 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
193194

194195
// Struct type not specified: create anon struct type from members.
195196
if (!structTy)
196-
structTy =
197-
getType<cir::StructType>(members, packed, cir::StructType::Struct,
198-
/*ast=*/nullptr);
197+
structTy = getType<cir::StructType>(members, packed, padded,
198+
cir::StructType::Struct,
199+
/*ast=*/nullptr);
199200

200201
// Return zero or anonymous constant struct.
201202
if (isZero)
@@ -205,6 +206,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
205206

206207
cir::ConstStructAttr getAnonConstStruct(mlir::ArrayAttr arrayAttr,
207208
bool packed = false,
209+
bool padded = false,
208210
mlir::Type ty = {}) {
209211
llvm::SmallVector<mlir::Type, 4> members;
210212
for (auto &f : arrayAttr) {
@@ -214,7 +216,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
214216
}
215217

216218
if (!ty)
217-
ty = getAnonStructTy(members, packed);
219+
ty = getAnonStructTy(members, packed, padded);
218220

219221
auto sTy = mlir::dyn_cast<cir::StructType>(ty);
220222
assert(sTy && "expected struct type");
@@ -434,15 +436,15 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
434436

435437
/// Get a CIR anonymous struct type.
436438
cir::StructType getAnonStructTy(llvm::ArrayRef<mlir::Type> members,
437-
bool packed = false,
439+
bool packed = false, bool padded = false,
438440
const clang::RecordDecl *ast = nullptr) {
439441
cir::ASTRecordDeclAttr astAttr = nullptr;
440442
auto kind = cir::StructType::RecordKind::Struct;
441443
if (ast) {
442444
astAttr = getAttr<cir::ASTRecordDeclAttr>(ast);
443445
kind = getRecordKind(ast->getTagKind());
444446
}
445-
return getType<cir::StructType>(members, packed, kind, astAttr);
447+
return getType<cir::StructType>(members, packed, padded, kind, astAttr);
446448
}
447449

448450
/// Get a CIR record kind from a AST declaration tag.
@@ -477,6 +479,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
477479
/// it with a different set of attributes, this method will crash.
478480
cir::StructType getCompleteStructTy(llvm::ArrayRef<mlir::Type> members,
479481
llvm::StringRef name, bool packed,
482+
bool padded,
480483
const clang::RecordDecl *ast) {
481484
const auto nameAttr = getStringAttr(name);
482485
cir::ASTRecordDeclAttr astAttr = nullptr;
@@ -487,19 +490,19 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
487490
}
488491

489492
// Create or get the struct.
490-
auto type =
491-
getType<cir::StructType>(members, nameAttr, packed, kind, astAttr);
493+
auto type = getType<cir::StructType>(members, nameAttr, packed, padded,
494+
kind, astAttr);
492495

493496
// Complete an incomplete struct or ensure the existing complete struct
494497
// matches the requested attributes.
495-
type.complete(members, packed, astAttr);
498+
type.complete(members, packed, padded, astAttr);
496499

497500
return type;
498501
}
499502

500503
cir::StructType
501504
getCompleteStructType(mlir::ArrayAttr fields, bool packed = false,
502-
llvm::StringRef name = "",
505+
bool padded = false, llvm::StringRef name = "",
503506
const clang::RecordDecl *ast = nullptr) {
504507
llvm::SmallVector<mlir::Type, 8> members;
505508
for (auto &attr : fields) {
@@ -508,9 +511,9 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
508511
}
509512

510513
if (name.empty())
511-
return getAnonStructTy(members, packed, ast);
514+
return getAnonStructTy(members, packed, padded, ast);
512515
else
513-
return getCompleteStructTy(members, name, packed, ast);
516+
return getCompleteStructTy(members, name, packed, padded, ast);
514517
}
515518

516519
cir::ArrayType getArrayType(mlir::Type eltType, unsigned size) {

clang/lib/CIR/CodeGen/CIRGenExprConst.cpp

+23-2
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ mlir::Attribute ConstantAggregateBuilder::buildFrom(
385385
CharUnits AlignedSize = Size.alignTo(Align);
386386

387387
bool Packed = false;
388+
bool Padded = false;
388389
ArrayRef<mlir::Attribute> UnpackedElems = Elems;
389390

390391
llvm::SmallVector<mlir::Attribute, 32> UnpackedElemStorage;
@@ -435,13 +436,13 @@ mlir::Attribute ConstantAggregateBuilder::buildFrom(
435436
auto &builder = CGM.getBuilder();
436437
auto arrAttr = mlir::ArrayAttr::get(builder.getContext(),
437438
Packed ? PackedElems : UnpackedElems);
438-
auto strType = builder.getCompleteStructType(arrAttr, Packed);
439439

440+
auto strType = builder.getCompleteStructType(arrAttr, Packed);
440441
if (auto desired = dyn_cast<cir::StructType>(DesiredTy))
441442
if (desired.isLayoutIdentical(strType))
442443
strType = desired;
443444

444-
return builder.getConstStructOrZeroAttr(arrAttr, Packed, strType);
445+
return builder.getConstStructOrZeroAttr(arrAttr, Packed, Padded, strType);
445446
}
446447

447448
void ConstantAggregateBuilder::condense(CharUnits Offset,
@@ -521,6 +522,9 @@ class ConstStructBuilder {
521522
const FieldDecl &Field, bool AllowOverwrite,
522523
CharUnits &SizeSoFar, bool &ZeroFieldSize);
523524

525+
bool ApplyZeroInitPadding(const ASTRecordLayout &Layout, bool AllowOverwrite,
526+
CharUnits SizeSoFar);
527+
524528
mlir::Attribute Finalize(QualType Ty);
525529
};
526530

@@ -715,6 +719,10 @@ bool ConstStructBuilder::Build(InitListExpr *ILE, bool AllowOverwrite) {
715719
}
716720
}
717721

722+
if (ZeroInitPadding &&
723+
!ApplyZeroInitPadding(Layout, AllowOverwrite, SizeSoFar))
724+
return false;
725+
718726
return true;
719727
}
720728

@@ -853,6 +861,19 @@ bool ConstStructBuilder::ApplyZeroInitPadding(
853861
return true;
854862
}
855863

864+
bool ConstStructBuilder::ApplyZeroInitPadding(const ASTRecordLayout &Layout,
865+
bool AllowOverwrite,
866+
CharUnits SizeSoFar) {
867+
CharUnits TotalSize = Layout.getSize();
868+
if (SizeSoFar < TotalSize) {
869+
if (!AppendBytes(SizeSoFar, computePadding(CGM, TotalSize - SizeSoFar),
870+
AllowOverwrite))
871+
return false;
872+
}
873+
SizeSoFar = TotalSize;
874+
return true;
875+
}
876+
856877
mlir::Attribute ConstStructBuilder::Finalize(QualType Type) {
857878
Type = Type.getNonReferenceType();
858879
RecordDecl *RD = Type->castAs<RecordType>()->getDecl();

0 commit comments

Comments
 (0)