Skip to content

Conversation

Andres-Salamanca
Copy link
Contributor

This PR adds support for the discrete bit-field layout.
It is the same as this PR: llvm/clangir#1860

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

llvmbot commented Aug 29, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: None (Andres-Salamanca)

Changes

This PR adds support for the discrete bit-field layout.
It is the same as this PR: llvm/clangir#1860


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

4 Files Affected:

  • (modified) clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h (+12)
  • (modified) clang/include/clang/CIR/MissingFeatures.h (-1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+49-1)
  • (added) clang/test/CIR/CodeGen/mms-bitfields.c (+65)
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 }

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.

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()) {
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 or two that encounter this condition?

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.

} HEADER;

#pragma pack(pop)

Copy link
Contributor

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?

Copy link
Contributor Author

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.

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.

3 participants