Add requested example, test, and address review comments

Andres-Salamanca · Andres-Salamanca · commit 47fbf708394b · 2025-07-09T15:42:05.000-05:00
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1684,8 +1684,12 @@ def SetBitfieldOp : CIR_Op<"set_bitfield"> {
     base record, a size of the storage, a size the bit field, an offset
     of the bit field and a sign. Returns a value being stored.
 
-    A unit attribute `volatile` can be used to indicate a volatile load of the
+    A unit attribute `volatile` can be used to indicate a volatile store of the
     bitfield.
+      ```mlir
+        cir.set_bitfield(#bfi, %0 : !cir.ptr<!u32i>, %1 : !s32i) {is_volatile}
+                                                                       -> !s32i
+      ```
 
     Example.
     Suppose we have a struct with multiple bitfields stored in
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -224,7 +224,7 @@ void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst,
       return;
     }
 
-    assert(dst.isBitField() && "NIY LValue type");
+    assert(dst.isBitField() && "Unknown LValue type");
     emitStoreThroughBitfieldLValue(src, dst);
     return;
 
@@ -332,6 +332,7 @@ mlir::Value CIRGenFunction::emitStoreThroughBitfieldLValue(RValue src,
   mlir::Type resLTy = convertTypeForMem(dst.getType());
   Address ptr = dst.getBitFieldAddress();
 
+  assert(!cir::MissingFeatures::armComputeVolatileBitfields());
   const bool useVolatile = false;
 
   mlir::Value dstAddr = dst.getAddress().getPointer();
@@ -1077,11 +1078,10 @@ LValue CIRGenFunction::emitBinaryOperatorLValue(const BinaryOperator *e) {
     LValue lv = emitLValue(e->getLHS());
 
     SourceLocRAIIObject loc{*this, getLoc(e->getSourceRange())};
-    if (lv.isBitField()) {
+    if (lv.isBitField())
       emitStoreThroughBitfieldLValue(rv, lv);
-    } else {
+    else
       emitStoreThroughLValue(rv, lv);
-    }
 
     if (getLangOpts().OpenMP) {
       cgm.errorNYI(e->getSourceRange(), "openmp");
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2385,6 +2385,24 @@ mlir::LogicalResult CIRToLLVMComplexImagOpLowering::matchAndRewrite(
   return mlir::success();
 }
 
+mlir::IntegerType computeBitfieldIntType(mlir::Type storageType,
+                                         mlir::MLIRContext *context,
+                                         unsigned &storageSize) {
+  return TypeSwitch<mlir::Type, mlir::IntegerType>(storageType)
+      .Case<cir::ArrayType>([&](cir::ArrayType atTy) {
+        storageSize = atTy.getSize() * 8;
+        return mlir::IntegerType::get(context, storageSize);
+      })
+      .Case<cir::IntType>([&](cir::IntType intTy) {
+        storageSize = intTy.getWidth();
+        return mlir::IntegerType::get(context, storageSize);
+      })
+      .Default([](mlir::Type) -> mlir::IntegerType {
+        llvm_unreachable(
+            "Either ArrayType or IntType expected for bitfields storage");
+      });
+}
+
 mlir::LogicalResult CIRToLLVMSetBitfieldOpLowering::matchAndRewrite(
     cir::SetBitfieldOp op, OpAdaptor adaptor,
     mlir::ConversionPatternRewriter &rewriter) const {
@@ -2400,19 +2418,7 @@ mlir::LogicalResult CIRToLLVMSetBitfieldOpLowering::matchAndRewrite(
   unsigned storageSize = 0;
 
   mlir::IntegerType intType =
-      TypeSwitch<mlir::Type, mlir::IntegerType>(storageType)
-          .Case<cir::ArrayType>([&](cir::ArrayType atTy) {
-            storageSize = atTy.getSize() * 8;
-            return mlir::IntegerType::get(context, storageSize);
-          })
-          .Case<cir::IntType>([&](cir::IntType intTy) {
-            storageSize = intTy.getWidth();
-            return mlir::IntegerType::get(context, storageSize);
-          })
-          .Default([](mlir::Type) -> mlir::IntegerType {
-            llvm_unreachable(
-                "Either ArrayType or IntType expected for bitfields storage");
-          });
+      computeBitfieldIntType(storageType, context, storageSize);
 
   mlir::Value srcVal = createIntCast(rewriter, adaptor.getSrc(), intType);
   unsigned srcWidth = storageSize;
@@ -2476,19 +2482,7 @@ mlir::LogicalResult CIRToLLVMGetBitfieldOpLowering::matchAndRewrite(
   unsigned storageSize = 0;
 
   mlir::IntegerType intType =
-      TypeSwitch<mlir::Type, mlir::IntegerType>(storageType)
-          .Case<cir::ArrayType>([&](cir::ArrayType atTy) {
-            storageSize = atTy.getSize() * 8;
-            return mlir::IntegerType::get(context, storageSize);
-          })
-          .Case<cir::IntType>([&](cir::IntType intTy) {
-            storageSize = intTy.getWidth();
-            return mlir::IntegerType::get(context, storageSize);
-          })
-          .Default([](mlir::Type) -> mlir::IntegerType {
-            llvm_unreachable(
-                "Either ArrayType or IntType expected for bitfields storage");
-          });
+      computeBitfieldIntType(storageType, context, storageSize);
 
   mlir::Value val = rewriter.create<mlir::LLVM::LoadOp>(
       op.getLoc(), intType, adaptor.getAddr(), 0, op.getIsVolatile());
diff --git a/clang/test/CIR/CodeGen/bitfields.c b/clang/test/CIR/CodeGen/bitfields.c
@@ -160,3 +160,82 @@ void store_field() {
 // OGCG:   [[TMP3:%.*]] = and i16 [[TMP2]], -32768
 // OGCG:   [[TMP4:%.*]] = or i16 [[TMP3]], 3
 // OGCG:   store i16 [[TMP4]], ptr [[TMP1]], align 4
+
+void store_bitfield_to_bitfield() {
+  S s;
+  s.a = s.c;
+}
+
+// CIR: cir.func {{.*@store_bitfield_to_bitfield}}
+// CIR:   [[TMP0:%.*]] = cir.alloca !rec_S, !cir.ptr<!rec_S>, ["s"] {alignment = 4 : i64}
+// CIR:   [[TMP1:%.*]] = cir.get_member [[TMP0]][0] {name = "c"} : !cir.ptr<!rec_S> -> !cir.ptr<!u64i>
+// CIR:   [[TMP2:%.*]] = cir.get_bitfield(#bfi_c, [[TMP1]] : !cir.ptr<!u64i>) -> !s32i
+// CIR:   [[TMP3:%.*]] = cir.get_member [[TMP0]][0] {name = "a"} : !cir.ptr<!rec_S> -> !cir.ptr<!u64i>
+// CIR:   [[TMP4:%.*]] = cir.set_bitfield(#bfi_a, [[TMP3]] : !cir.ptr<!u64i>, [[TMP2]] : !s32i) -> !s32i
+
+// LLVM: define dso_local void @store_bitfield_to_bitfield()
+// LLVM:  [[TMP0:%.*]] = alloca %struct.S, i64 1, align 4
+// LLVM:  [[TMP1:%.*]] = getelementptr %struct.S, ptr [[TMP0]], i32 0, i32 0
+// LLVM:  [[TMP2:%.*]] = load i64, ptr [[TMP1]], align 8
+// LLVM:  [[TMP3:%.*]] = shl i64 [[TMP2]], 15
+// LLVM:  [[TMP4:%.*]] = ashr i64 [[TMP3]], 47
+// LLVM:  [[TMP5:%.*]] = trunc i64 [[TMP4]] to i32
+// LLVM:  [[TMP6:%.*]] = getelementptr %struct.S, ptr [[TMP0]], i32 0, i32 0
+// LLVM:  [[TMP7:%.*]] = zext i32 [[TMP5]] to i64
+// LLVM:  [[TMP8:%.*]] = load i64, ptr [[TMP6]], align 8
+// LLVM:  [[TMP9:%.*]] = and i64 [[TMP7]], 15
+// LLVM:  [[TMP10:%.*]] = and i64 [[TMP8]], -16
+// LLVM:  [[TMP11:%.*]] = or i64 [[TMP10]], [[TMP9]]
+// LLVM:  store i64 [[TMP11]], ptr [[TMP6]], align 8
+// LLVM:  [[TMP12:%.*]] = shl i64 [[TMP9]], 60
+// LLVM:  [[TMP13:%.*]] = ashr i64 [[TMP12]], 60
+// LLVM:  [[TMP15:%.*]] = trunc i64 [[TMP13]] to i32
+
+// OGCG: define dso_local void @store_bitfield_to_bitfield()
+// OGCG:  [[TMP0:%.*]] = alloca %struct.S, align 4
+// OGCG:  [[TMP1:%.*]] = load i64, ptr [[TMP0]], align 4
+// OGCG:  [[TMP2:%.*]] = shl i64 [[TMP1]], 15
+// OGCG:  [[TMP3:%.*]] = ashr i64 [[TMP2]], 47
+// OGCG:  [[TMP4:%.*]] = trunc i64 [[TMP3]] to i32
+// OGCG:  [[TMP5:%.*]] = zext i32 [[TMP4]] to i64
+// OGCG:  [[TMP6:%.*]] = load i64, ptr [[TMP0]], align 4
+// OGCG:  [[TMP7:%.*]] = and i64 [[TMP5]], 15
+// OGCG:  [[TMP8:%.*]] = and i64 [[TMP6]], -16
+// OGCG:  [[TMP9:%.*]] = or i64 [[TMP8]], [[TMP7]]
+// OGCG:  store i64 [[TMP9]], ptr [[TMP0]], align 4
+// OGCG:  [[TMP10:%.*]] = shl i64 %bf.value, 60
+// OGCG:  [[TMP11:%.*]] = ashr i64 [[TMP10]], 60
+// OGCG:  [[TMP12:%.*]] = trunc i64 [[TMP11]] to i32
+
+typedef struct {
+  int a : 30;
+  int volatile b : 8;
+  int c;
+} V;
+
+void set_volatile(V* v) {
+  v->b = 3;
+}
+//CIR: cir.func dso_local @set_volatile
+//CIR:   [[TMP0:%.*]] = cir.alloca !cir.ptr<!rec_V>, !cir.ptr<!cir.ptr<!rec_V>>, ["v", init] {alignment = 8 : i64}
+//CIR:   [[TMP1:%.*]] = cir.const #cir.int<3> : !s32i
+//CIR:   [[TMP2:%.*]] = cir.load align(8) [[TMP0]] : !cir.ptr<!cir.ptr<!rec_V>>, !cir.ptr<!rec_V>
+//CIR:   [[TMP3:%.*]] = cir.get_member [[TMP2]][0] {name = "b"} : !cir.ptr<!rec_V> -> !cir.ptr<!u64i>
+//CIR:   [[TMP4:%.*]] = cir.set_bitfield(#bfi_b, [[TMP3]] : !cir.ptr<!u64i>, [[TMP1]] : !s32i) {is_volatile} -> !s32i
+
+// LLVM: define dso_local void @set_volatile
+// LLVM:   [[TMP0:%.*]] = alloca ptr, i64 1, align 8
+// LLVM:   [[TMP1:%.*]] = load ptr, ptr [[TMP0]], align 8
+// LLVM:   [[TMP2:%.*]] = getelementptr %struct.V, ptr [[TMP1]], i32 0, i32 0
+// LLVM:   [[TMP3:%.*]] = load volatile i64, ptr [[TMP2]], align 8
+// LLVM:   [[TMP4:%.*]] = and i64 [[TMP3]], -1095216660481
+// LLVM:   [[TMP5:%.*]] = or i64 [[TMP4]], 12884901888
+// LLVM:   store volatile i64 [[TMP5]], ptr [[TMP2]], align 8
+
+// OGCG: define dso_local void @set_volatile
+// OGCG:   [[TMP0:%.*]] = alloca ptr, align 8
+// OGCG:   [[TMP1:%.*]] = load ptr, ptr [[TMP0]], align 8
+// OGCG:   [[TMP2:%.*]] = load volatile i64, ptr [[TMP1]], align 4
+// OGCG:   [[TMP3:%.*]] = and i64 [[TMP2]], -1095216660481
+// OGCG:   [[TMP4:%.*]] = or i64 [[TMP3]], 12884901888
+// OGCG:   store volatile i64 [[TMP4]], ptr [[TMP1]], align 4
