Skip to content

Commit

Permalink
[CIR][CodeGen] Fix packed structures (#839)
Browse files Browse the repository at this point in the history
Consider the following code snippet `test.c`:
```

typedef struct {
    char b;
    int c;
} D;

typedef struct {
    D e;
    int f;
} E;

void f1() {
    E a = {};
}
```
When emitting the CIR using `bin/clang test.c -Xclang -fclangir -Xclang
-emit-cir -S -o -` the current implementation gets:
```
NYI
UNREACHABLE executed at ~/clangir/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp:338!
 ```
This is only one of the many tests where this happens.

Comparing the implementations of `CIR/CodeGen/CIRRecordLayoutBuilder.cpp` and clang's codegen `lib/CodeGen/CGRecordLayoutBuilder.cpp`, there is some padding missing for packed structures, and some alignments that need to be corrected.

This PR also updates 2 existing tests. In the first test, `structural-binding.cpp`, I updated some `cir.get_member` indexes. In the second test, `packed-structs.c`, I updated the `cir` layout for the structure, and added more tests. I have compared the changes I made in the tests to the original clang codegen and everything seems fine.
  • Loading branch information
bruteforceboy authored and lanza committed Nov 3, 2024
1 parent ff0a297 commit 20bcbb9
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 17 deletions.
35 changes: 32 additions & 3 deletions clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ struct CIRRecordLowering final {

/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
void insertPadding();

void computeVolatileBitfields();
void accumulateBases();
Expand Down Expand Up @@ -294,6 +296,7 @@ void CIRRecordLowering::lower(bool nonVirtualBaseType) {

members.push_back(StorageInfo(Size, getUIntNType(8)));
determinePacked(nonVirtualBaseType);
insertPadding();
members.pop_back();

fillOutputFields();
Expand Down Expand Up @@ -657,6 +660,33 @@ void CIRRecordLowering::determinePacked(bool NVBaseType) {
members.back().data = getUIntNType(astContext.toBits(Alignment));
}

void CIRRecordLowering::insertPadding() {
std::vector<std::pair<CharUnits, CharUnits>> Padding;
CharUnits Size = CharUnits::Zero();
for (std::vector<MemberInfo>::const_iterator Member = members.begin(),
MemberEnd = members.end();
Member != MemberEnd; ++Member) {
if (!Member->data)
continue;
CharUnits Offset = Member->offset;
assert(Offset >= Size);
// Insert padding if we need to.
if (Offset !=
Size.alignTo(isPacked ? CharUnits::One() : getAlignment(Member->data)))
Padding.push_back(std::make_pair(Size, Offset - Size));
Size = Offset + getSize(Member->data);
}
if (Padding.empty())
return;
// Add the padding to the Members list and sort it.
for (std::vector<std::pair<CharUnits, CharUnits>>::const_iterator
Pad = Padding.begin(),
PadEnd = Padding.end();
Pad != PadEnd; ++Pad)
members.push_back(StorageInfo(Pad->first, getByteArrayType(Pad->second)));
llvm::stable_sort(members);
}

std::unique_ptr<CIRGenRecordLayout>
CIRGenTypes::computeRecordLayout(const RecordDecl *D,
mlir::cir::StructType *Ty) {
Expand All @@ -674,9 +704,8 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *D,
CIRRecordLowering baseBuilder(*this, D, /*Packed=*/builder.isPacked);
baseBuilder.lower(/*NonVirtualBaseType=*/true);
auto baseIdentifier = getRecordTypeName(D, ".base");
BaseTy =
Builder.getCompleteStructTy(baseBuilder.fieldTypes, baseIdentifier,
/*packed=*/false, D);
BaseTy = Builder.getCompleteStructTy(
baseBuilder.fieldTypes, baseIdentifier, baseBuilder.isPacked, D);
// TODO(cir): add something like addRecordTypeName

// BaseTy and Ty must agree on their packedness for getCIRFieldNo to work
Expand Down
7 changes: 6 additions & 1 deletion clang/lib/CIR/Dialect/IR/CIRDataLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ StructLayout::StructLayout(mlir::cir::StructType ST, const CIRDataLayout &DL)

assert(!::cir::MissingFeatures::recordDeclIsPacked() &&
"Cannot identify packed structs");
const llvm::Align TyAlign = DL.getABITypeAlign(Ty);
const llvm::Align TyAlign =
ST.getPacked() ? llvm::Align(1) : DL.getABITypeAlign(Ty);

// Add padding if necessary to align the data element properly.
// Currently the only structure with scalable size will be the homogeneous
Expand Down Expand Up @@ -171,6 +172,10 @@ llvm::Align CIRDataLayout::getAlignment(mlir::Type Ty, bool abiOrPref) const {
if (::cir::MissingFeatures::recordDeclIsPacked() && abiOrPref)
llvm_unreachable("NYI");

auto stTy = llvm::dyn_cast<mlir::cir::StructType>(Ty);
if (stTy && stTy.getPacked() && abiOrPref)
return llvm::Align(1);

// Get the layout annotation... which is lazily created on demand.
const StructLayout *Layout =
getStructLayout(llvm::cast<mlir::cir::StructType>(Ty));
Expand Down
110 changes: 102 additions & 8 deletions clang/test/CIR/CodeGen/packed-structs.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s
// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t.ll
// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s

#pragma pack(1)

Expand All @@ -20,18 +22,110 @@ typedef struct {
} __attribute__((aligned(2))) C;


// CHECK: !ty_A = !cir.struct<struct "A" packed {!s32i, !s8i}>
// CHECK: !ty_C = !cir.struct<struct "C" packed {!s32i, !s8i}>
// CHECK: !ty_B = !cir.struct<struct "B" packed {!s32i, !s8i, !cir.array<!ty_A x 6>}>
// CIR: !ty_A = !cir.struct<struct "A" packed {!s32i, !s8i}>
// CIR: !ty_C = !cir.struct<struct "C" packed {!s32i, !s8i, !u8i}>
// CIR: !ty_D = !cir.struct<struct "D" packed {!s8i, !u8i, !s32i}
// CIR: !ty_F = !cir.struct<struct "F" packed {!s64i, !s8i}
// CIR: !ty_E = !cir.struct<struct "E" packed {!ty_D
// CIR: !ty_G = !cir.struct<struct "G" {!ty_F
// CIR: !ty_H = !cir.struct<struct "H" {!s32i, !ty_anon2E0_
// CIR: !ty_B = !cir.struct<struct "B" packed {!s32i, !s8i, !cir.array<!ty_A x 6>}>
// CIR: !ty_I = !cir.struct<struct "I" packed {!s8i, !ty_H
// CIR: !ty_J = !cir.struct<struct "J" packed {!s8i, !s8i, !s8i, !s8i, !ty_I

// CHECK: cir.func {{.*@foo()}}
// CHECK: %0 = cir.alloca !ty_A, !cir.ptr<!ty_A>, ["a"] {alignment = 1 : i64}
// CHECK: %1 = cir.alloca !ty_B, !cir.ptr<!ty_B>, ["b"] {alignment = 1 : i64}
// CHECK: %2 = cir.alloca !ty_C, !cir.ptr<!ty_C>, ["c"] {alignment = 2 : i64}
// LLVM: %struct.A = type <{ i32, i8 }>
// LLVM: %struct.B = type <{ i32, i8, [6 x %struct.A] }>
// LLVM: %struct.C = type <{ i32, i8, i8 }>
// LLVM: %struct.E = type <{ %struct.D, i32 }>
// LLVM: %struct.D = type <{ i8, i8, i32 }>
// LLVM: %struct.G = type { %struct.F, i8 }
// LLVM: %struct.F = type <{ i64, i8 }>
// LLVM: %struct.J = type <{ i8, i8, i8, i8, %struct.I, i32 }>
// LLVM: %struct.I = type <{ i8, %struct.H }>
// LLVM: %struct.H = type { i32, %union.anon.{{.*}} }

// CIR: cir.func {{.*@foo()}}
// CIR: {{.*}} = cir.alloca !ty_A, !cir.ptr<!ty_A>, ["a"] {alignment = 1 : i64}
// CIR: {{.*}} = cir.alloca !ty_B, !cir.ptr<!ty_B>, ["b"] {alignment = 1 : i64}
// CIR: {{.*}} = cir.alloca !ty_C, !cir.ptr<!ty_C>, ["c"] {alignment = 2 : i64}

// LLVM: {{.*}} = alloca %struct.A, i64 1, align 1
// LLVM: {{.*}} = alloca %struct.B, i64 1, align 1
// LLVM: {{.*}} = alloca %struct.C, i64 1, align 2
void foo() {
A a;
B b;
C c;
}

#pragma pack(2)

typedef struct {
char b;
int c;
} D;

typedef struct {
D e;
int f;
} E;

// CIR: cir.func {{.*@f1()}}
// CIR: {{.*}} = cir.alloca !ty_E, !cir.ptr<!ty_E>, ["a"] {alignment = 2 : i64}

// LLVM: {{.*}} = alloca %struct.E, i64 1, align 2
void f1() {
E a = {};
}

#pragma pack(1)

typedef struct {
long b;
char c;
} F;

typedef struct {
F e;
char f;
} G;

// CIR: cir.func {{.*@f2()}}
// CIR: {{.*}} = cir.alloca !ty_G, !cir.ptr<!ty_G>, ["a"] {alignment = 1 : i64}

// LLVM: {{.*}} = alloca %struct.G, i64 1, align 1
void f2() {
G a = {};
}

#pragma pack(1)

typedef struct {
int d0;
union {
char null;
int val;
} value;
} H;

typedef struct {
char t;
H d;
} I;

typedef struct {
char a0;
char a1;
char a2;
char a3;
I c;
int a;
} J;

// CIR: cir.func {{.*@f3()}}
// CIR: {{.*}} = cir.alloca !ty_J, !cir.ptr<!ty_J>, ["a"] {alignment = 1 : i64}

// LLVM: {{.*}} = alloca %struct.J, i64 1, align 1
void f3() {
J a = {0};
}
10 changes: 5 additions & 5 deletions clang/test/CIR/CodeGen/structural-binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,19 @@ void f(A &a) {
// CIR: %[[a:.*]] = cir.load %1 : !cir.ptr<!cir.ptr<!ty_A>>, !cir.ptr<!ty_A>
// CIR: {{.*}} = cir.get_member %[[a]][0] {name = "a"} : !cir.ptr<!ty_A> -> !cir.ptr<!ty_B>
// CIR: %[[a:.*]] = cir.load %1 : !cir.ptr<!cir.ptr<!ty_A>>, !cir.ptr<!ty_A>
// CIR: {{.*}} = cir.get_member %[[a]][1] {name = "b"} : !cir.ptr<!ty_A> -> !cir.ptr<!s32i>
// CIR: {{.*}} = cir.get_member %[[a]][2] {name = "b"} : !cir.ptr<!ty_A> -> !cir.ptr<!s32i>
// CIR: %[[a:.*]] = cir.load %1 : !cir.ptr<!cir.ptr<!ty_A>>, !cir.ptr<!ty_A>
// CIR: {{.*}} = cir.get_member %[[a]][2] {name = "c"} : !cir.ptr<!ty_A> -> !cir.ptr<!s8i>
// CIR: {{.*}} = cir.get_member %[[a]][3] {name = "c"} : !cir.ptr<!ty_A> -> !cir.ptr<!s8i>
// LLVM: {{.*}} = getelementptr %struct.A, ptr {{.*}}, i32 0, i32 0
// LLVM: {{.*}} = getelementptr %struct.A, ptr {{.*}}, i32 0, i32 1
// LLVM: {{.*}} = getelementptr %struct.A, ptr {{.*}}, i32 0, i32 2
// LLVM: {{.*}} = getelementptr %struct.A, ptr {{.*}}, i32 0, i32 3

auto [x2, y2, z2] = a;
(x2, y2, z2);
// CIR: cir.call @_ZN1AC1ERKS_(%2, {{.*}}) : (!cir.ptr<!ty_A>, !cir.ptr<!ty_A>) -> ()
// CIR: {{.*}} = cir.get_member %2[0] {name = "a"} : !cir.ptr<!ty_A> -> !cir.ptr<!ty_B>
// CIR: {{.*}} = cir.get_member %2[1] {name = "b"} : !cir.ptr<!ty_A> -> !cir.ptr<!s32i>
// CIR: {{.*}} = cir.get_member %2[2] {name = "c"} : !cir.ptr<!ty_A> -> !cir.ptr<!s8i>
// CIR: {{.*}} = cir.get_member %2[2] {name = "b"} : !cir.ptr<!ty_A> -> !cir.ptr<!s32i>
// CIR: {{.*}} = cir.get_member %2[3] {name = "c"} : !cir.ptr<!ty_A> -> !cir.ptr<!s8i>

// for the rest, just expect the codegen does't crash
auto &&[x3, y3, z3] = a;
Expand Down

0 comments on commit 20bcbb9

Please sign in to comment.