-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[CIR] Add support for bitfields in unions #145096
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
Conversation
@llvm/pr-subscribers-clang Author: None (Andres-Salamanca) ChangesThis PR introduces support for bitfields inside unions in CIR. Full diff: https://github.com/llvm/llvm-project/pull/145096.diff 2 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 8dbf1b36a93b2..3d50a537e676b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -655,11 +655,14 @@ void CIRRecordLowering::lowerUnion() {
// locate the "most appropriate" storage type.
for (const FieldDecl *field : recordDecl->fields()) {
mlir::Type fieldType;
- if (field->isBitField())
- cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
- "bitfields in lowerUnion");
- else
+ if (field->isBitField()) {
+ if (field->isZeroLengthBitField())
+ continue;
+ fieldType = getBitfieldStorageType(field->getBitWidthValue());
+ setBitFieldInfo(field, CharUnits::Zero(), fieldType);
+ } else {
fieldType = getStorageType(field);
+ }
// This maps a field to its index. For unions, the index is always 0.
fieldIdxMap[field->getCanonicalDecl()] = 0;
diff --git a/clang/test/CIR/CodeGen/bitfield-union.c b/clang/test/CIR/CodeGen/bitfield-union.c
new file mode 100644
index 0000000000000..72e14c5fb71f3
--- /dev/null
+++ b/clang/test/CIR/CodeGen/bitfield-union.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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 -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 -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+typedef union {
+ int x;
+ int y : 4;
+ int z : 8;
+} demo;
+
+// CIR: !rec_demo = !cir.record<union "demo" {!s32i, !u8i, !u8i}>
+// LLVM: %union.demo = type { i32 }
+// OGCG: %union.demo = type { i32 }
+demo d;
+
+
|
@llvm/pr-subscribers-clangir Author: None (Andres-Salamanca) ChangesThis PR introduces support for bitfields inside unions in CIR. Full diff: https://github.com/llvm/llvm-project/pull/145096.diff 2 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 8dbf1b36a93b2..3d50a537e676b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -655,11 +655,14 @@ void CIRRecordLowering::lowerUnion() {
// locate the "most appropriate" storage type.
for (const FieldDecl *field : recordDecl->fields()) {
mlir::Type fieldType;
- if (field->isBitField())
- cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
- "bitfields in lowerUnion");
- else
+ if (field->isBitField()) {
+ if (field->isZeroLengthBitField())
+ continue;
+ fieldType = getBitfieldStorageType(field->getBitWidthValue());
+ setBitFieldInfo(field, CharUnits::Zero(), fieldType);
+ } else {
fieldType = getStorageType(field);
+ }
// This maps a field to its index. For unions, the index is always 0.
fieldIdxMap[field->getCanonicalDecl()] = 0;
diff --git a/clang/test/CIR/CodeGen/bitfield-union.c b/clang/test/CIR/CodeGen/bitfield-union.c
new file mode 100644
index 0000000000000..72e14c5fb71f3
--- /dev/null
+++ b/clang/test/CIR/CodeGen/bitfield-union.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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 -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 -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+typedef union {
+ int x;
+ int y : 4;
+ int z : 8;
+} demo;
+
+// CIR: !rec_demo = !cir.record<union "demo" {!s32i, !u8i, !u8i}>
+// LLVM: %union.demo = type { i32 }
+// OGCG: %union.demo = type { i32 }
+demo d;
+
+
|
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.
Thanks for the incremental approach, LGTM.
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.
This looks pretty straight-forward. I just have a request for an additional test case.
// CIR: !rec_demo = !cir.record<union "demo" {!s32i, !u8i, !u8i}> | ||
// LLVM: %union.demo = type { i32 } | ||
// OGCG: %union.demo = type { i32 } | ||
demo d; |
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 with a zero-length bitfield? I'm not sure why anyone would put that in a union, but there's code for it so we should test it.
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
This PR introduces support for bitfields inside unions in CIR.
This PR introduces support for bitfields inside unions in CIR.
This PR introduces support for bitfields inside unions in CIR.
This PR introduces support for bitfields inside unions in CIR.