Skip to content

Conversation

@Andres-Salamanca
Copy link
Contributor

This PR adds support for -ffine-grained-bitfield-accesses. I reused the tests from classic CodeGen, available here:
https://github.com/llvm/llvm-project/blob/c2c881fcc85e0c2d7a050b0199d4dadf8f556b9e/clang/test/CodeGenCXX/finegrain-bitfield-access.cpp

We produce almost exactly the same codegen, except when returning a variable: we emit an extra variable to hold the return value, whereas classic CodeGen does not. Also, the GEP instructions use slightly different syntax compared to classic CodeGen.

@Andres-Salamanca Andres-Salamanca requested a review from lanza as a code owner July 25, 2025 19:51
@Andres-Salamanca Andres-Salamanca changed the title Fine bits [CIR] Add support for -ffine-grained-bitfield-accesses Jul 25, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jul 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2025

@llvm/pr-subscribers-clang

Author: None (Andres-Salamanca)

Changes

This PR adds support for -ffine-grained-bitfield-accesses. I reused the tests from classic CodeGen, available here:
https://github.com/llvm/llvm-project/blob/c2c881fcc85e0c2d7a050b0199d4dadf8f556b9e/clang/test/CodeGenCXX/finegrain-bitfield-access.cpp

We produce almost exactly the same codegen, except when returning a variable: we emit an extra variable to hold the return value, whereas classic CodeGen does not. Also, the GEP instructions use slightly different syntax compared to classic CodeGen.


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

3 Files Affected:

  • (modified) clang/include/clang/CIR/MissingFeatures.h (-1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+6-9)
  • (added) clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp (+271)
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 182e4b6784d2f..cc3b0f5b02d35 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -222,7 +222,6 @@ struct MissingFeatures {
   static bool moduleNameHash() { return false; }
   static bool msabi() { return false; }
   static bool needsGlobalCtorDtor() { return false; }
-  static bool nonFineGrainedBitfields() { return false; }
   static bool objCBlocks() { return false; }
   static bool objCGC() { return false; }
   static bool objCLifetime() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 05e8848884f7e..404bfef1ca210 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -432,21 +432,18 @@ CIRRecordLowering::accumulateBitFields(RecordDecl::field_iterator field,
             bestEnd = field;
             bestClipped = false;
           }
-          if (barrier) {
+          if (barrier)
             // The next field is a barrier that we cannot merge across.
             installBest = true;
-          } else if (cirGenTypes.getCGModule()
-                         .getCodeGenOpts()
-                         .FineGrainedBitfieldAccesses) {
-            assert(!cir::MissingFeatures::nonFineGrainedBitfields());
-            cirGenTypes.getCGModule().errorNYI(field->getSourceRange(),
-                                               "NYI FineGrainedBitfield");
-          } else {
+          else if (cirGenTypes.getCGModule()
+                       .getCodeGenOpts()
+                       .FineGrainedBitfieldAccesses)
+            installBest = true;
+          else
             // Otherwise, we're not installing. Update the bit size
             // of the current span to go all the way to limitOffset, which is
             // the (aligned) offset of next bitfield to consider.
             bitSizeSinceBegin = astContext.toBits(limitOffset - beginOffset);
-          }
         }
       }
     }
diff --git a/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp b/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp
new file mode 100644
index 0000000000000..930b0a9c70059
--- /dev/null
+++ b/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -ffine-grained-bitfield-accesses %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 -ffine-grained-bitfield-accesses %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 -ffine-grained-bitfield-accesses %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+struct S1 {
+  unsigned f1:2;
+  unsigned f2:6;
+  unsigned f3:8;
+  unsigned f4:4;
+  unsigned f5:8;
+};
+
+// CIR-DAG: !rec_S1 = !cir.record<struct "S1" {!u8i, !u8i, !u16i}>
+// LLVM-DAG: %struct.S1 = type { i8, i8, i16 }
+// OGCG-DAG: %struct.S1 = type { i8, i8, i16 }
+
+struct S2 {
+  unsigned long f1:16;
+  unsigned long f2:16;
+  unsigned long f3:6;
+};
+
+// CIR-DAG: !rec_S2 = !cir.record<struct "S2" padded {!u16i, !u16i, !u8i, !cir.array<!u8i x 3>}>
+// LLVM-DAG: %struct.S2 = type { i16, i16, i8, [3 x i8] }
+// OGCG-DAG: %struct.S2 = type { i16, i16, i8, [3 x i8] }
+
+struct S3 {
+  unsigned long f1:14;
+  unsigned long f2:18;
+  unsigned long f3:32;
+};
+
+// CIR-DAG: !rec_S3 = !cir.record<struct "S3" {!u32i, !u32i}>
+// LLVM-DAG: %struct.S3 = type { i32, i32 }
+// OGCG-DAG: %struct.S3 = type { i32, i32 }
+
+S1 a1;
+S2 a2;
+S3 a3;
+
+unsigned read8_1() {
+  return a1.f3;
+}
+
+// CIR-LABEL: @_Z7read8_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member %1[1] {name = "f3"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u8i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(1) (#bfi_f3, [[MEMBER]] : !cir.ptr<!u8i>) -> !u32i
+// CIR: cir.store [[BITFI]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z7read8_1v
+// LLVM:  [[MEMBER:%.*]] = load i8, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 1), align 1
+// LLVM:  [[BFCAST:%.*]] = zext i8 [[MEMBER]] to i32
+// LLVM:  store i32 [[BFCAST]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z7read8_1v
+// OGCG: [[BFLOAD:%.*]] = load i8, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 1), align 1
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i8 [[BFLOAD]] to i32
+// OGCG-NEXT: ret i32 [[BFCAST]]
+
+void write8_1() {
+  a1.f3 = 3;
+}
+
+// CIR-LABEL: @_Z8write8_1v
+// CIR: [[CONST3:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[INT3:%.*]] = cir.cast(integral, [[CONST3]] : !s32i), !u32i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u8i>
+// CIR: cir.set_bitfield align(1) (#bfi_f3, [[MEMBER]] : !cir.ptr<!u8i>, [[INT3]] : !u32i) -> !u32i
+
+// LLVM-LABEL: @_Z8write8_1v
+// LLVM:  store i8 3, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 1), align 1
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z8write8_1v
+// OGCG: store i8 3, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 1), align 1
+// OGCG-NEXT: ret void
+
+unsigned read8_2() {
+
+  return a1.f5;
+}
+
+// CIR-LABEL: @_Z7read8_2v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[2] {name = "f5"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(2) (#bfi_f5, [[MEMBER]] : !cir.ptr<!u16i>) -> !u32i
+// CIR: cir.store [[BITFI]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z7read8_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFLSHR:%.*]] = lshr i16 [[BFLOAD]], 4
+// LLVM:  [[BFCLEAR:%.*]] = and i16 [[BFLSHR]], 255
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFCLEAR]] to i32
+// LLVM:  store i32 [[BFCAST]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z7read8_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: [[BFLSHR:%.*]] = lshr i16 [[BFLOAD]], 4
+// OGCG-NEXT: [[BFCLEAR:%.*]] = and i16 [[BFLSHR]], 255
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFCLEAR]] to i32
+// OGCG-NEXT: ret i32 [[BFCAST]]
+
+void write8_2() {
+  a1.f5 = 3;
+}
+
+// CIR-LABEL: @_Z8write8_2v
+// CIR: [[CONST3:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[INT3:%.*]] = cir.cast(integral, [[CONST3]] : !s32i), !u32i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[2] {name = "f5"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(2) (#bfi_f5, %3 : !cir.ptr<!u16i>, {{.*}} : !u32i) -> !u32i
+
+// LLVM-LABEL: @_Z8write8_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFCLEAR:%.*]] = and i16 [[BFLOAD]], -4081
+// LLVM:  [[BFSET:%.*]] = or i16 [[BFCLEAR]], 48
+// LLVM:  store i16 [[BFSET]], ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z8write8_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: [[BFCLEAR:%.*]] = and i16 [[BFLOAD]], -4081
+// OGCG-NEXT: [[BFSET:%.*]] = or i16 [[BFCLEAR]], 48
+// OGCG-NEXT: store i16 [[BFSET]], ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: ret void
+
+unsigned read16_1() {
+  return a2.f1;
+}
+
+// CIR-LABEL: @_Z8read16_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[0] {name = "f1"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(8) (#bfi_f1, [[MEMBER]] : !cir.ptr<!u16i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read16_1v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr {{.*}}, align 8
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// LLVM:  [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM:  store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read16_1v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr {{.*}}, align 8
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 [[BFCAST]] to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+unsigned read16_2() {
+  return a2.f2;
+}
+
+// CIR-LABEL: @_Z8read16_2v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f2"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(2) (#bfi_f2, [[MEMBER]] : !cir.ptr<!u16i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read16_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// LLVM:  [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM:  store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read16_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S2, ptr {{.*}}, i32 0, i32 1), align 2
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 [[BFCAST]] to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+void write16_1() {
+  a2.f1 = 5;
+}
+
+// CIR-LABEL: @_Z9write16_1v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]]  = cir.get_member {{.*}}[0] {name = "f1"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(8) (#bfi_f1, [[MEMBER]] : !cir.ptr<!u16i>, [[INT5]] : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write16_1v
+// LLVM:  store i16 5, ptr {{.*}}, align 8
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z9write16_1v
+// OGCG: store i16 5, ptr {{.*}}, align 8
+// OGCG-NEXT: ret void
+
+void write16_2() {
+
+  a2.f2 = 5;
+}
+
+// CIR-LABEL: @_Z9write16_2v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f2"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(2) (#bfi_f2, [[MEMBER]] : !cir.ptr<!u16i>, {{.*}} : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write16_2v
+// LLVM: store i16 5, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM: ret void
+
+// OGCG-LABEL: @_Z9write16_2v
+// OGCG: store i16 5, ptr getelementptr inbounds nuw (%struct.S2, ptr {{.*}}, i32 0, i32 1), align 2
+// OGCG-NEXT: ret void
+
+unsigned read32_1() {
+
+  return a3.f3;
+}
+// CIR-LABEL: @_Z8read32_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S3> -> !cir.ptr<!u32i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(4) (#bfi_f3_1, [[MEMBER]] : !cir.ptr<!u32i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read32_1v
+// LLVM: [[BFLOAD:%.*]] = load i32, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 4), align 4
+// LLVM: [[BFCAST:%.*]] = zext i32 [[BFLOAD]] to i64
+// LLVM: [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM: store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM: [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM: ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read32_1v
+// OGCG: [[BFLOAD:%.*]] = load i32, ptr getelementptr inbounds nuw (%struct.S3, ptr {{.*}}, i32 0, i32 1), align 4
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i32 %bf.load to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 %bf.cast to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+void write32_1() {
+  a3.f3 = 5;
+}
+
+// CIR-LABEL: @_Z9write32_1v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S3> -> !cir.ptr<!u32i>
+// CIR: cir.set_bitfield align(4) (#bfi_f3_1, [[MEMBER]] : !cir.ptr<!u32i>, [[INT5]] : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write32_1v
+// LLVM:  store i32 5, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 4), align 4
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z9write32_1v
+// OGCG: store i32 5, ptr getelementptr inbounds nuw (%struct.S3, ptr {{.*}}, i32 0, i32 1), align 4
+// OGCG-NEXT: ret void

@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2025

@llvm/pr-subscribers-clangir

Author: None (Andres-Salamanca)

Changes

This PR adds support for -ffine-grained-bitfield-accesses. I reused the tests from classic CodeGen, available here:
https://github.com/llvm/llvm-project/blob/c2c881fcc85e0c2d7a050b0199d4dadf8f556b9e/clang/test/CodeGenCXX/finegrain-bitfield-access.cpp

We produce almost exactly the same codegen, except when returning a variable: we emit an extra variable to hold the return value, whereas classic CodeGen does not. Also, the GEP instructions use slightly different syntax compared to classic CodeGen.


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

3 Files Affected:

  • (modified) clang/include/clang/CIR/MissingFeatures.h (-1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+6-9)
  • (added) clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp (+271)
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 182e4b6784d2f..cc3b0f5b02d35 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -222,7 +222,6 @@ struct MissingFeatures {
   static bool moduleNameHash() { return false; }
   static bool msabi() { return false; }
   static bool needsGlobalCtorDtor() { return false; }
-  static bool nonFineGrainedBitfields() { return false; }
   static bool objCBlocks() { return false; }
   static bool objCGC() { return false; }
   static bool objCLifetime() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 05e8848884f7e..404bfef1ca210 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -432,21 +432,18 @@ CIRRecordLowering::accumulateBitFields(RecordDecl::field_iterator field,
             bestEnd = field;
             bestClipped = false;
           }
-          if (barrier) {
+          if (barrier)
             // The next field is a barrier that we cannot merge across.
             installBest = true;
-          } else if (cirGenTypes.getCGModule()
-                         .getCodeGenOpts()
-                         .FineGrainedBitfieldAccesses) {
-            assert(!cir::MissingFeatures::nonFineGrainedBitfields());
-            cirGenTypes.getCGModule().errorNYI(field->getSourceRange(),
-                                               "NYI FineGrainedBitfield");
-          } else {
+          else if (cirGenTypes.getCGModule()
+                       .getCodeGenOpts()
+                       .FineGrainedBitfieldAccesses)
+            installBest = true;
+          else
             // Otherwise, we're not installing. Update the bit size
             // of the current span to go all the way to limitOffset, which is
             // the (aligned) offset of next bitfield to consider.
             bitSizeSinceBegin = astContext.toBits(limitOffset - beginOffset);
-          }
         }
       }
     }
diff --git a/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp b/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp
new file mode 100644
index 0000000000000..930b0a9c70059
--- /dev/null
+++ b/clang/test/CIR/CodeGen/finegrain-bitfield-access.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -ffine-grained-bitfield-accesses %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 -ffine-grained-bitfield-accesses %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 -ffine-grained-bitfield-accesses %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+struct S1 {
+  unsigned f1:2;
+  unsigned f2:6;
+  unsigned f3:8;
+  unsigned f4:4;
+  unsigned f5:8;
+};
+
+// CIR-DAG: !rec_S1 = !cir.record<struct "S1" {!u8i, !u8i, !u16i}>
+// LLVM-DAG: %struct.S1 = type { i8, i8, i16 }
+// OGCG-DAG: %struct.S1 = type { i8, i8, i16 }
+
+struct S2 {
+  unsigned long f1:16;
+  unsigned long f2:16;
+  unsigned long f3:6;
+};
+
+// CIR-DAG: !rec_S2 = !cir.record<struct "S2" padded {!u16i, !u16i, !u8i, !cir.array<!u8i x 3>}>
+// LLVM-DAG: %struct.S2 = type { i16, i16, i8, [3 x i8] }
+// OGCG-DAG: %struct.S2 = type { i16, i16, i8, [3 x i8] }
+
+struct S3 {
+  unsigned long f1:14;
+  unsigned long f2:18;
+  unsigned long f3:32;
+};
+
+// CIR-DAG: !rec_S3 = !cir.record<struct "S3" {!u32i, !u32i}>
+// LLVM-DAG: %struct.S3 = type { i32, i32 }
+// OGCG-DAG: %struct.S3 = type { i32, i32 }
+
+S1 a1;
+S2 a2;
+S3 a3;
+
+unsigned read8_1() {
+  return a1.f3;
+}
+
+// CIR-LABEL: @_Z7read8_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member %1[1] {name = "f3"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u8i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(1) (#bfi_f3, [[MEMBER]] : !cir.ptr<!u8i>) -> !u32i
+// CIR: cir.store [[BITFI]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z7read8_1v
+// LLVM:  [[MEMBER:%.*]] = load i8, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 1), align 1
+// LLVM:  [[BFCAST:%.*]] = zext i8 [[MEMBER]] to i32
+// LLVM:  store i32 [[BFCAST]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z7read8_1v
+// OGCG: [[BFLOAD:%.*]] = load i8, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 1), align 1
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i8 [[BFLOAD]] to i32
+// OGCG-NEXT: ret i32 [[BFCAST]]
+
+void write8_1() {
+  a1.f3 = 3;
+}
+
+// CIR-LABEL: @_Z8write8_1v
+// CIR: [[CONST3:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[INT3:%.*]] = cir.cast(integral, [[CONST3]] : !s32i), !u32i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u8i>
+// CIR: cir.set_bitfield align(1) (#bfi_f3, [[MEMBER]] : !cir.ptr<!u8i>, [[INT3]] : !u32i) -> !u32i
+
+// LLVM-LABEL: @_Z8write8_1v
+// LLVM:  store i8 3, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 1), align 1
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z8write8_1v
+// OGCG: store i8 3, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 1), align 1
+// OGCG-NEXT: ret void
+
+unsigned read8_2() {
+
+  return a1.f5;
+}
+
+// CIR-LABEL: @_Z7read8_2v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[2] {name = "f5"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(2) (#bfi_f5, [[MEMBER]] : !cir.ptr<!u16i>) -> !u32i
+// CIR: cir.store [[BITFI]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z7read8_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFLSHR:%.*]] = lshr i16 [[BFLOAD]], 4
+// LLVM:  [[BFCLEAR:%.*]] = and i16 [[BFLSHR]], 255
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFCLEAR]] to i32
+// LLVM:  store i32 [[BFCAST]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z7read8_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: [[BFLSHR:%.*]] = lshr i16 [[BFLOAD]], 4
+// OGCG-NEXT: [[BFCLEAR:%.*]] = and i16 [[BFLSHR]], 255
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFCLEAR]] to i32
+// OGCG-NEXT: ret i32 [[BFCAST]]
+
+void write8_2() {
+  a1.f5 = 3;
+}
+
+// CIR-LABEL: @_Z8write8_2v
+// CIR: [[CONST3:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[INT3:%.*]] = cir.cast(integral, [[CONST3]] : !s32i), !u32i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[2] {name = "f5"} : !cir.ptr<!rec_S1> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(2) (#bfi_f5, %3 : !cir.ptr<!u16i>, {{.*}} : !u32i) -> !u32i
+
+// LLVM-LABEL: @_Z8write8_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFCLEAR:%.*]] = and i16 [[BFLOAD]], -4081
+// LLVM:  [[BFSET:%.*]] = or i16 [[BFCLEAR]], 48
+// LLVM:  store i16 [[BFSET]], ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z8write8_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: [[BFCLEAR:%.*]] = and i16 [[BFLOAD]], -4081
+// OGCG-NEXT: [[BFSET:%.*]] = or i16 [[BFCLEAR]], 48
+// OGCG-NEXT: store i16 [[BFSET]], ptr getelementptr inbounds nuw (%struct.S1, ptr {{.*}}, i32 0, i32 2), align 2
+// OGCG-NEXT: ret void
+
+unsigned read16_1() {
+  return a2.f1;
+}
+
+// CIR-LABEL: @_Z8read16_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[0] {name = "f1"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(8) (#bfi_f1, [[MEMBER]] : !cir.ptr<!u16i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read16_1v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr {{.*}}, align 8
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// LLVM:  [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM:  store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read16_1v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr {{.*}}, align 8
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 [[BFCAST]] to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+unsigned read16_2() {
+  return a2.f2;
+}
+
+// CIR-LABEL: @_Z8read16_2v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f2"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(2) (#bfi_f2, [[MEMBER]] : !cir.ptr<!u16i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read16_2v
+// LLVM:  [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM:  [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// LLVM:  [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM:  store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM:  [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM:  ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read16_2v
+// OGCG: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (%struct.S2, ptr {{.*}}, i32 0, i32 1), align 2
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i16 [[BFLOAD]] to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 [[BFCAST]] to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+void write16_1() {
+  a2.f1 = 5;
+}
+
+// CIR-LABEL: @_Z9write16_1v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]]  = cir.get_member {{.*}}[0] {name = "f1"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(8) (#bfi_f1, [[MEMBER]] : !cir.ptr<!u16i>, [[INT5]] : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write16_1v
+// LLVM:  store i16 5, ptr {{.*}}, align 8
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z9write16_1v
+// OGCG: store i16 5, ptr {{.*}}, align 8
+// OGCG-NEXT: ret void
+
+void write16_2() {
+
+  a2.f2 = 5;
+}
+
+// CIR-LABEL: @_Z9write16_2v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f2"} : !cir.ptr<!rec_S2> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield align(2) (#bfi_f2, [[MEMBER]] : !cir.ptr<!u16i>, {{.*}} : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write16_2v
+// LLVM: store i16 5, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
+// LLVM: ret void
+
+// OGCG-LABEL: @_Z9write16_2v
+// OGCG: store i16 5, ptr getelementptr inbounds nuw (%struct.S2, ptr {{.*}}, i32 0, i32 1), align 2
+// OGCG-NEXT: ret void
+
+unsigned read32_1() {
+
+  return a3.f3;
+}
+// CIR-LABEL: @_Z8read32_1v
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S3> -> !cir.ptr<!u32i>
+// CIR: [[BITFI:%.*]] = cir.get_bitfield align(4) (#bfi_f3_1, [[MEMBER]] : !cir.ptr<!u32i>) -> !u64i
+// CIR: [[BFCAST:%.*]] = cir.cast(integral, [[BITFI]] : !u64i), !u32i
+// CIR: cir.store [[BFCAST]], {{.*}} : !u32i, !cir.ptr<!u32i>
+// CIR: [[RET:%.*]] = cir.load {{.*}} : !cir.ptr<!u32i>, !u32i
+// CIR: cir.return [[RET]] : !u32i
+
+// LLVM-LABEL: @_Z8read32_1v
+// LLVM: [[BFLOAD:%.*]] = load i32, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 4), align 4
+// LLVM: [[BFCAST:%.*]] = zext i32 [[BFLOAD]] to i64
+// LLVM: [[BF:%.*]] = trunc i64 [[BFCAST]] to i32
+// LLVM: store i32 [[BF]], ptr {{.*}}, align 4
+// LLVM: [[RET:%.*]] = load i32, ptr {{.*}}, align 4
+// LLVM: ret i32 [[RET]]
+
+// OGCG-LABEL: @_Z8read32_1v
+// OGCG: [[BFLOAD:%.*]] = load i32, ptr getelementptr inbounds nuw (%struct.S3, ptr {{.*}}, i32 0, i32 1), align 4
+// OGCG-NEXT: [[BFCAST:%.*]] = zext i32 %bf.load to i64
+// OGCG-NEXT: [[RET:%.*]] = trunc i64 %bf.cast to i32
+// OGCG-NEXT: ret i32 [[RET]]
+
+void write32_1() {
+  a3.f3 = 5;
+}
+
+// CIR-LABEL: @_Z9write32_1v
+// CIR: [[CONST5:%.*]] = cir.const #cir.int<5> : !s32i
+// CIR: [[INT5:%.*]] = cir.cast(integral, [[CONST5]] : !s32i), !u64i
+// CIR: [[MEMBER:%.*]] = cir.get_member {{.*}}[1] {name = "f3"} : !cir.ptr<!rec_S3> -> !cir.ptr<!u32i>
+// CIR: cir.set_bitfield align(4) (#bfi_f3_1, [[MEMBER]] : !cir.ptr<!u32i>, [[INT5]] : !u64i) -> !u64i
+// CIR: cir.return
+
+// LLVM-LABEL: @_Z9write32_1v
+// LLVM:  store i32 5, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 4), align 4
+// LLVM:  ret void
+
+// OGCG-LABEL: @_Z9write32_1v
+// OGCG: store i32 5, ptr getelementptr inbounds nuw (%struct.S3, ptr {{.*}}, i32 0, i32 1), align 4
+// OGCG-NEXT: ret void

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 great. I have just one nit.

bestClipped = false;
}
if (barrier) {
if (barrier)
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep the braces here. Even though the language syntax doesn't require them, the multi-line clauses with comments make this meet the coding standard guidelines for braces.

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

// CIR: cir.return [[RET]] : !u32i

// LLVM-LABEL: @_Z8read16_2v
// LLVM: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why we're generating i8-based GEPs here, while classic codegen uses the structure type. I guess that's not related to this PR, and it's semantically equivalent, but it surprises me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I noticed that too it is doing pointer arithmetic with, which is semantically equivalent. Do you think it’s worth looking into making it use the structure type instead? I can take a look if it seems worthwhile.

Copy link
Member

Choose a reason for hiding this comment

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

It's worth tracking the difference in an issue or at least with a comment so we remember to look at it at some point

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

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.

LGTM

// CIR: cir.return [[RET]] : !u32i

// LLVM-LABEL: @_Z8read16_2v
// LLVM: [[BFLOAD:%.*]] = load i16, ptr getelementptr inbounds nuw (i8, ptr {{.*}}, i64 2), align 2
Copy link
Member

Choose a reason for hiding this comment

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

It's worth tracking the difference in an issue or at least with a comment so we remember to look at it at some point

@Andres-Salamanca Andres-Salamanca merged commit a5db2c2 into llvm:main Jul 29, 2025
9 checks passed
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