Skip to content

[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

Merged
merged 2 commits into from
Jun 24, 2025

Conversation

Andres-Salamanca
Copy link
Contributor

This PR introduces support for bitfields inside unions in CIR.

@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-clang

Author: None (Andres-Salamanca)

Changes

This PR introduces support for bitfields inside unions in CIR.


Full diff: https://github.com/llvm/llvm-project/pull/145096.diff

2 Files Affected:

  • (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+7-4)
  • (added) clang/test/CIR/CodeGen/bitfield-union.c (+19)
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; 
+
+

@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-clangir

Author: None (Andres-Salamanca)

Changes

This PR introduces support for bitfields inside unions in CIR.


Full diff: https://github.com/llvm/llvm-project/pull/145096.diff

2 Files Affected:

  • (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+7-4)
  • (added) clang/test/CIR/CodeGen/bitfield-union.c (+19)
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; 
+
+

Copy link
Member

@bcardosolopes bcardosolopes left a 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.

Copy link
Contributor

@andykaylor andykaylor left a 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;
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bcardosolopes bcardosolopes merged commit 8763d29 into llvm:main Jun 24, 2025
7 checks passed
DrSergei pushed a commit to DrSergei/llvm-project that referenced this pull request Jun 24, 2025
This PR introduces support for bitfields inside unions in CIR.
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
This PR introduces support for bitfields inside unions in CIR.
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
This PR introduces support for bitfields inside unions in CIR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants