-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[CIR] Add support for discrete bit-field #156085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: None (Andres-Salamanca) ChangesThis PR adds support for the discrete bit-field layout. Full diff: https://github.com/llvm/llvm-project/pull/156085.diff 4 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h b/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
index ecc681ee310e3..4da435e0ea8f7 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
+++ b/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h
@@ -68,6 +68,18 @@ class CIRDataLayout {
return llvm::alignTo(getTypeStoreSize(ty), getABITypeAlign(ty).value());
}
+ /// Returns the offset in bits between successive objects of the
+ /// specified type, including alignment padding; always a multiple of 8.
+ ///
+ /// If Ty is a scalable vector type, the scalable property will be set and
+ /// the runtime size will be a positive integer multiple of the base size.
+ ///
+ /// This is the amount that alloca reserves for this type. For example,
+ /// returns 96 or 128 for x86_fp80, depending on alignment.
+ llvm::TypeSize getTypeAllocSizeInBits(mlir::Type ty) const {
+ return 8 * getTypeAllocSize(ty);
+ }
+
llvm::TypeSize getTypeSizeInBits(mlir::Type ty) const;
};
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index d7a2e49ec162a..9b00e07722896 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -149,7 +149,6 @@ struct MissingFeatures {
static bool cxxabiUseARMGuardVarABI() { return false; }
static bool cxxabiAppleARM64CXXABI() { return false; }
static bool cxxabiStructorImplicitParam() { return false; }
- static bool isDiscreteBitFieldABI() { return false; }
// Address class
static bool addressOffset() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 1764967329969..06be03224c671 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -96,6 +96,16 @@ struct CIRRecordLowering final {
/// Helper function to check if the target machine is BigEndian.
bool isBigEndian() const { return astContext.getTargetInfo().isBigEndian(); }
+ /// The Microsoft bitfield layout rule allocates discrete storage
+ /// units of the field's formal type and only combines adjacent
+ /// fields of the same formal type. We want to emit a layout with
+ /// these discrete storage units instead of combining them into a
+ /// continuous run.
+ bool isDiscreteBitFieldABI() {
+ return astContext.getTargetInfo().getCXXABI().isMicrosoft() ||
+ recordDecl->isMsStruct(astContext);
+ }
+
CharUnits bitsToCharUnits(uint64_t bitOffset) {
return astContext.toCharUnitsFromBits(bitOffset);
}
@@ -306,7 +316,45 @@ void CIRRecordLowering::fillOutputFields() {
RecordDecl::field_iterator
CIRRecordLowering::accumulateBitFields(RecordDecl::field_iterator field,
RecordDecl::field_iterator fieldEnd) {
- assert(!cir::MissingFeatures::isDiscreteBitFieldABI());
+ if (isDiscreteBitFieldABI()) {
+ // run stores the first element of the current run of bitfields. fieldEnd is
+ // used as a special value to note that we don't have a current run. A
+ // bitfield run is a contiguous collection of bitfields that can be stored
+ // in the same storage block. Zero-sized bitfields and bitfields that would
+ // cross an alignment boundary break a run and start a new one.
+ RecordDecl::field_iterator run = fieldEnd;
+ // tail is the offset of the first bit off the end of the current run. It's
+ // used to determine if the ASTRecordLayout is treating these two bitfields
+ // as contiguous. StartBitOffset is offset of the beginning of the Run.
+ uint64_t startBitOffset, tail = 0;
+ for (; field != fieldEnd && field->isBitField(); ++field) {
+ // Zero-width bitfields end runs.
+ if (field->isZeroLengthBitField()) {
+ run = fieldEnd;
+ continue;
+ }
+ uint64_t bitOffset = getFieldBitOffset(*field);
+ mlir::Type type = cirGenTypes.convertTypeForMem(field->getType());
+ // If we don't have a run yet, or don't live within the previous run's
+ // allocated storage then we allocate some storage and start a new run.
+ if (run == fieldEnd || bitOffset >= tail) {
+ run = field;
+ startBitOffset = bitOffset;
+ tail = startBitOffset + dataLayout.getTypeAllocSizeInBits(type);
+ // Add the storage member to the record. This must be added to the
+ // record before the bitfield members so that it gets laid out before
+ // the bitfields it contains get laid out.
+ members.push_back(
+ makeStorageInfo(bitsToCharUnits(startBitOffset), type));
+ }
+ // Bitfields get the offset of their storage but come afterward and remain
+ // there after a stable sort.
+ members.push_back(MemberInfo(bitsToCharUnits(startBitOffset),
+ MemberInfo::InfoKind::Field, nullptr,
+ *field));
+ }
+ return field;
+ }
CharUnits regSize =
bitsToCharUnits(astContext.getTargetInfo().getRegisterWidth());
diff --git a/clang/test/CIR/CodeGen/mms-bitfields.c b/clang/test/CIR/CodeGen/mms-bitfields.c
new file mode 100644
index 0000000000000..071746822e745
--- /dev/null
+++ b/clang/test/CIR/CodeGen/mms-bitfields.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -mms-bitfields -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -mms-bitfields -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -mms-bitfields -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+struct s1 {
+ int f32 : 2;
+ long long f64 : 30;
+} s1;
+
+// CIR-DAG: !rec_s1 = !cir.record<struct "s1" {!s32i, !s64i}>
+// LLVM-DAG: %struct.s1 = type { i32, i64 }
+// OGCG-DAG: %struct.s1 = type { i32, i64 }
+
+struct s2 {
+ int a : 24;
+ char b;
+ int c : 30;
+} Clip;
+
+// CIR-DAG: !rec_s2 = !cir.record<struct "s2" {!s32i, !s8i, !s32i}>
+// LLVM-DAG: %struct.s2 = type { i32, i8, i32 }
+// OGCG-DAG: %struct.s2 = type { i32, i8, i32 }
+
+#pragma pack (push,1)
+
+struct Inner {
+ unsigned int A : 1;
+ unsigned int B : 1;
+ unsigned int C : 1;
+ unsigned int D : 30;
+} Inner;
+
+#pragma pack (pop)
+
+// CIR-DAG: !rec_Inner = !cir.record<struct "Inner" {!u32i, !u32i}>
+// LLVM-DAG: %struct.Inner = type { i32, i32 }
+// OGCG-DAG: %struct.Inner = type { i32, i32 }
+
+#pragma pack(push, 1)
+
+union HEADER {
+ struct A {
+ int : 3; // Bits 2:0
+ int a : 9; // Bits 11:3
+ int : 12; // Bits 23:12
+ int b : 17; // Bits 40:24
+ int : 7; // Bits 47:41
+ int c : 4; // Bits 51:48
+ int : 4; // Bits 55:52
+ int d : 3; // Bits 58:56
+ int : 5; // Bits 63:59
+ } Bits;
+} HEADER;
+
+#pragma pack(pop)
+
+// CIR-DAG: !rec_A = !cir.record<struct "A" {!s32i, !s32i, !s32i}>
+// CIR-DAG: !rec_HEADER = !cir.record<union "HEADER" {!rec_A}>
+// LLVM-DAG: %struct.A = type { i32, i32, i32 }
+// LLVM-DAG: %union.HEADER = type { %struct.A }
+// OGCG-DAG: %struct.A = type { i32, i32, i32 }
+// OGCG-DAG: %union.HEADER = type { %struct.A }
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, with just a couple of test requests.
uint64_t startBitOffset, tail = 0; | ||
for (; field != fieldEnd && field->isBitField(); ++field) { | ||
// Zero-width bitfields end runs. | ||
if (field->isZeroLengthBitField()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test case or two that encounter this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} HEADER; | ||
|
||
#pragma pack(pop) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clang/test/CodeGen
version of this test has global variables with initializers for Inner and HEADER. Can you add those here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can’t add those tests because we currently don’t have upstream ILE support #155663. I saw that this is in the process of being upstreamed, but bitfields were left out.
This PR adds support for the discrete bit-field layout.
It is the same as this PR: llvm/clangir#1860