[CIR] Upstream new SetBitfieldOp for handling C and C++ struct bitfields #147609
+324
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: None (Andres-Salamanca) ChangesThis PR upstreams the Full diff: https://github.com/llvm/llvm-project/pull/147609.diff 9 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 6529f1386599c..37800ea82e8af 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1669,6 +1669,90 @@ def GetGlobalOp : CIR_Op<"get_global",
}];
}
+//===----------------------------------------------------------------------===//
+// SetBitfieldOp
+//===----------------------------------------------------------------------===//
+
+def SetBitfieldOp : CIR_Op<"set_bitfield"> {
+ let summary = "Set the value of a bitfield member";
+ let description = [{
+ The `cir.set_bitfield` operation provides a store-like access to
+ a bit field of a record.
+
+ It expects an address of a storage where to store, a type of the storage,
+ a value being stored, a name of a bit field, a pointer to the storage in the
+ 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
+ bitfield.
+
+ Example.
+ Suppose we have a struct with multiple bitfields stored in
+ different storages. The `cir.set_bitfield` operation sets the value
+ of the bitfield.
+ ```C++
+ typedef struct {
+ int a : 4;
+ int b : 27;
+ int c : 17;
+ int d : 2;
+ int e : 15;
+ } S;
+
+ void store_bitfield(S& s) {
+ s.e = 3;
+ }
+ ```
+
+ ```mlir
+ // 'e' is in the storage with the index 1
+ !record_type = !cir.record<struct "S" packed padded {!u64i, !u16i,
+ !cir.array<!u8i x 2>} #cir.record.decl.ast>
+ #bfi_e = #cir.bitfield_info<name = "e", storage_type = !u16i, size = 15,
+ offset = 0, is_signed = true>
+
+ %1 = cir.const #cir.int<3> : !s32i
+ %2 = cir.load %0 : !cir.ptr<!cir.ptr<!record_type>>, !cir.ptr<!record_type>
+ %3 = cir.get_member %2[1] {name = "e"} : !cir.ptr<!record_type>
+ -> !cir.ptr<!u16i>
+ %4 = cir.set_bitfield(#bfi_e, %3 : !cir.ptr<!u16i>, %1 : !s32i) -> !s32i
+ ```
+ }];
+
+ let arguments = (ins
+ Arg<CIR_PointerType, "the address to store the value", [MemWrite]>:$addr,
+ CIR_AnyType:$src,
+ BitfieldInfoAttr:$bitfield_info,
+ UnitAttr:$is_volatile
+ );
+
+ let results = (outs CIR_IntType:$result);
+
+ let assemblyFormat = [{ `(`$bitfield_info`,` $addr`:`qualified(type($addr))`,`
+ $src`:`type($src) `)` attr-dict `->` type($result) }];
+
+ let builders = [
+ OpBuilder<(ins "mlir::Type":$type,
+ "mlir::Value":$addr,
+ "mlir::Type":$storage_type,
+ "mlir::Value":$src,
+ "llvm::StringRef":$name,
+ "unsigned":$size,
+ "unsigned":$offset,
+ "bool":$is_signed,
+ "bool":$is_volatile
+ ),
+ [{
+ BitfieldInfoAttr info =
+ BitfieldInfoAttr::get($_builder.getContext(),
+ name, storage_type,
+ size, offset, is_signed);
+ build($_builder, $_state, type, addr, src, info, is_volatile);
+ }]>
+ ];
+}
+
//===----------------------------------------------------------------------===//
// GetBitfieldOp
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index 0b33f6c7d03b7..d95ea36a5e0d0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -394,6 +394,15 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
return createGlobal(module, loc, uniqueName, type, linkage);
}
+ mlir::Value createSetBitfield(mlir::Location loc, mlir::Type resultType,
+ mlir::Value dstAddr, mlir::Type storageType,
+ mlir::Value src, const CIRGenBitFieldInfo &info,
+ bool isLvalueVolatile, bool useVolatile) {
+ return create<cir::SetBitfieldOp>(loc, resultType, dstAddr, storageType,
+ src, info.name, info.size, info.offset,
+ info.isSigned, isLvalueVolatile);
+ }
+
mlir::Value createGetBitfield(mlir::Location loc, mlir::Type resultType,
mlir::Value addr, mlir::Type storageType,
const CIRGenBitFieldInfo &info,
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 68d7f1f5bca48..5e6f63d9395c6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -224,6 +224,10 @@ void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst,
return;
}
+ assert(dst.isBitField() && "NIY LValue type");
+ emitStoreThroughBitfieldLValue(src, dst);
+ return;
+
cgm.errorNYI(dst.getPointer().getLoc(),
"emitStoreThroughLValue: non-simple lvalue");
return;
@@ -321,9 +325,20 @@ void CIRGenFunction::emitStoreOfScalar(mlir::Value value, Address addr,
mlir::Value CIRGenFunction::emitStoreThroughBitfieldLValue(RValue src,
LValue dst) {
- assert(!cir::MissingFeatures::bitfields());
- cgm.errorNYI("bitfields");
- return {};
+
+ assert(!cir::MissingFeatures::armComputeVolatileBitfields());
+
+ const CIRGenBitFieldInfo &info = dst.getBitFieldInfo();
+ mlir::Type resLTy = convertTypeForMem(dst.getType());
+ Address ptr = dst.getBitFieldAddress();
+
+ const bool useVolatile = false;
+
+ mlir::Value dstAddr = dst.getAddress().getPointer();
+
+ return builder.createSetBitfield(dstAddr.getLoc(), resLTy, dstAddr,
+ ptr.getElementType(), src.getValue(), info,
+ dst.isVolatileQualified(), useVolatile);
}
RValue CIRGenFunction::emitLoadOfBitfieldLValue(LValue lv, SourceLocation loc) {
@@ -1063,10 +1078,10 @@ LValue CIRGenFunction::emitBinaryOperatorLValue(const BinaryOperator *e) {
SourceLocRAIIObject loc{*this, getLoc(e->getSourceRange())};
if (lv.isBitField()) {
- cgm.errorNYI(e->getSourceRange(), "bitfields");
- return {};
+ emitStoreThroughBitfieldLValue(rv, lv);
+ } else {
+ emitStoreThroughLValue(rv, lv);
}
- emitStoreThroughLValue(rv, lv);
if (getLangOpts().OpenMP) {
cgm.errorNYI(e->getSourceRange(), "openmp");
diff --git a/clang/lib/CIR/CodeGen/CIRGenValue.h b/clang/lib/CIR/CodeGen/CIRGenValue.h
index e1b0f805a7b21..0a6dba5e80a62 100644
--- a/clang/lib/CIR/CodeGen/CIRGenValue.h
+++ b/clang/lib/CIR/CodeGen/CIRGenValue.h
@@ -186,6 +186,8 @@ class LValue {
bool isBitField() const { return lvType == BitField; }
bool isVolatile() const { return quals.hasVolatile(); }
+ bool isVolatileQualified() const { return quals.hasVolatile(); }
+
unsigned getVRQualifiers() const {
return quals.getCVRQualifiers() & ~clang::Qualifiers::Const;
}
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 5ac42b6a63b09..5364a515fb384 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2049,6 +2049,7 @@ void ConvertCIRToLLVMPass::runOnOperation() {
CIRToLLVMGetGlobalOpLowering,
CIRToLLVMGetMemberOpLowering,
CIRToLLVMSelectOpLowering,
+ CIRToLLVMSetBitfieldOpLowering,
CIRToLLVMShiftOpLowering,
CIRToLLVMStackRestoreOpLowering,
CIRToLLVMStackSaveOpLowering,
@@ -2384,6 +2385,82 @@ mlir::LogicalResult CIRToLLVMComplexImagOpLowering::matchAndRewrite(
return mlir::success();
}
+mlir::LogicalResult CIRToLLVMSetBitfieldOpLowering::matchAndRewrite(
+ cir::SetBitfieldOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const {
+ mlir::OpBuilder::InsertionGuard guard(rewriter);
+ rewriter.setInsertionPoint(op);
+
+ cir::BitfieldInfoAttr info = op.getBitfieldInfo();
+ uint64_t size = info.getSize();
+ uint64_t offset = info.getOffset();
+ mlir::Type storageType = info.getStorageType();
+ mlir::MLIRContext *context = storageType.getContext();
+
+ 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");
+ });
+
+ mlir::Value srcVal = createIntCast(rewriter, adaptor.getSrc(), intType);
+ unsigned srcWidth = storageSize;
+ mlir::Value resultVal = srcVal;
+
+ if (storageSize != size) {
+ assert(storageSize > size && "Invalid bitfield size.");
+
+ mlir::Value val = rewriter.create<mlir::LLVM::LoadOp>(
+ op.getLoc(), intType, adaptor.getAddr(), /* alignment */ 0,
+ op.getIsVolatile());
+
+ srcVal =
+ createAnd(rewriter, srcVal, llvm::APInt::getLowBitsSet(srcWidth, size));
+ resultVal = srcVal;
+ srcVal = createShL(rewriter, srcVal, offset);
+
+ // Mask out the original value.
+ val = createAnd(rewriter, val,
+ ~llvm::APInt::getBitsSet(srcWidth, offset, offset + size));
+
+ // Or together the unchanged values and the source value.
+ srcVal = rewriter.create<mlir::LLVM::OrOp>(op.getLoc(), val, srcVal);
+ }
+
+ rewriter.create<mlir::LLVM::StoreOp>(op.getLoc(), srcVal, adaptor.getAddr(),
+ /* alignment */ 0, op.getIsVolatile());
+
+ mlir::Type resultTy = getTypeConverter()->convertType(op.getType());
+
+ if (info.getIsSigned()) {
+ assert(size <= storageSize);
+ unsigned highBits = storageSize - size;
+
+ if (highBits) {
+ resultVal = createShL(rewriter, resultVal, highBits);
+ resultVal = createAShR(rewriter, resultVal, highBits);
+ }
+ }
+
+ resultVal = createIntCast(rewriter, resultVal,
+ mlir::cast<mlir::IntegerType>(resultTy),
+ info.getIsSigned());
+
+ rewriter.replaceOp(op, resultVal);
+ return mlir::success();
+}
+
mlir::LogicalResult CIRToLLVMGetBitfieldOpLowering::matchAndRewrite(
cir::GetBitfieldOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index d9fb91066317b..6c26d10269ea2 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -513,6 +513,16 @@ class CIRToLLVMComplexImagOpLowering
mlir::ConversionPatternRewriter &) const override;
};
+class CIRToLLVMSetBitfieldOpLowering
+ : public mlir::OpConversionPattern<cir::SetBitfieldOp> {
+public:
+ using mlir::OpConversionPattern<cir::SetBitfieldOp>::OpConversionPattern;
+
+ mlir::LogicalResult
+ matchAndRewrite(cir::SetBitfieldOp op, OpAdaptor,
+ mlir::ConversionPatternRewriter &) const override;
+};
+
class CIRToLLVMGetBitfieldOpLowering
: public mlir::OpConversionPattern<cir::GetBitfieldOp> {
public:
diff --git a/clang/test/CIR/CodeGen/bitfields.c b/clang/test/CIR/CodeGen/bitfields.c
index 6eb753c5cc3d2..ccb9cf0616a1e 100644
--- a/clang/test/CIR/CodeGen/bitfields.c
+++ b/clang/test/CIR/CodeGen/bitfields.c
@@ -134,3 +134,29 @@ unsigned int load_field_unsigned(A* s) {
//OGCG: [[TMP4:%.*]] = lshr i16 [[TMP3]], 3
//OGCG: [[TMP5:%.*]] = and i16 [[TMP4]], 15
//OGCG: [[TMP6:%.*]] = zext i16 [[TMP5]] to i32
+
+void store_field() {
+ S s;
+ s.e = 3;
+}
+// CIR: cir.func {{.*@store_field}}
+// CIR: [[TMP0:%.*]] = cir.alloca !rec_S, !cir.ptr<!rec_S>
+// CIR: [[TMP1:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[TMP2:%.*]] = cir.get_member [[TMP0]][1] {name = "e"} : !cir.ptr<!rec_S> -> !cir.ptr<!u16i>
+// CIR: cir.set_bitfield(#bfi_e, [[TMP2]] : !cir.ptr<!u16i>, [[TMP1]] : !s32i)
+
+// LLVM: define dso_local void @store_field()
+// LLVM: [[TMP0:%.*]] = alloca %struct.S, i64 1, align 4
+// LLVM: [[TMP1:%.*]] = getelementptr %struct.S, ptr [[TMP0]], i32 0, i32 1
+// LLVM: [[TMP2:%.*]] = load i16, ptr [[TMP1]], align 2
+// LLVM: [[TMP3:%.*]] = and i16 [[TMP2]], -32768
+// LLVM: [[TMP4:%.*]] = or i16 [[TMP3]], 3
+// LLVM: store i16 [[TMP4]], ptr [[TMP1]], align 2
+
+// OGCG: define dso_local void @store_field()
+// OGCG: [[TMP0:%.*]] = alloca %struct.S, align 4
+// OGCG: [[TMP1:%.*]] = getelementptr inbounds nuw %struct.S, ptr [[TMP0]], i32 0, i32 1
+// OGCG: [[TMP2:%.*]] = load i16, ptr [[TMP1]], align 4
+// OGCG: [[TMP3:%.*]] = and i16 [[TMP2]], -32768
+// OGCG: [[TMP4:%.*]] = or i16 [[TMP3]], 3
+// OGCG: store i16 [[TMP4]], ptr [[TMP1]], align 4
diff --git a/clang/test/CIR/CodeGen/bitfields.cpp b/clang/test/CIR/CodeGen/bitfields.cpp
index a4d58b5cadcec..f8f41942b2a40 100644
--- a/clang/test/CIR/CodeGen/bitfields.cpp
+++ b/clang/test/CIR/CodeGen/bitfields.cpp
@@ -58,3 +58,28 @@ int load_field(S* s) {
// OGCG: [[TMP3:%.*]] = shl i64 [[TMP2]], 15
// OGCG: [[TMP4:%.*]] = ashr i64 [[TMP3]], 47
// OGCG: [[TMP5:%.*]] = trunc i64 [[TMP4]] to i32
+
+void store_field() {
+ S s;
+ s.a = 3;
+}
+// CIR: cir.func dso_local @_Z11store_field
+// CIR: [[TMP0:%.*]] = cir.alloca !rec_S, !cir.ptr<!rec_S>
+// CIR: [[TMP1:%.*]] = cir.const #cir.int<3> : !s32i
+// CIR: [[TMP2:%.*]] = cir.get_member [[TMP0]][0] {name = "a"} : !cir.ptr<!rec_S> -> !cir.ptr<!u64i>
+// CIR: cir.set_bitfield(#bfi_a, [[TMP2]] : !cir.ptr<!u64i>, [[TMP1]] : !s32i)
+
+// LLVM: define dso_local void @_Z11store_fieldv
+// 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:%.*]] = and i64 [[TMP2]], -16
+// LLVM: [[TMP4:%.*]] = or i64 [[TMP3]], 3
+// LLVM: store i64 [[TMP4]], ptr [[TMP1]], align 8
+
+// OGCG: define dso_local void @_Z11store_fieldv()
+// OGCG: [[TMP0:%.*]] = alloca %struct.S, align 4
+// OGCG: [[TMP1:%.*]] = load i64, ptr [[TMP0]], align 4
+// OGCG: [[TMP2:%.*]] = and i64 [[TMP1]], -16
+// OGCG: [[TMP3:%.*]] = or i64 [[TMP2]], 3
+// OGCG: store i64 [[TMP3]], ptr [[TMP0]], align 4
diff --git a/clang/test/CIR/CodeGen/bitfields_be.c b/clang/test/CIR/CodeGen/bitfields_be.c
index e839bc2b9698d..6133927b67d21 100644
--- a/clang/test/CIR/CodeGen/bitfields_be.c
+++ b/clang/test/CIR/CodeGen/bitfields_be.c
@@ -42,3 +42,73 @@ int init(S* s) {
//OGCG: [[TMP2:%.*]] = load i32, ptr [[TMP1]], align 4
//OGCG: [[TMP3:%.*]] = shl i32 [[TMP2]], 15
//OGCG: [[TMP4:%.*]] = ashr i32 [[TMP3]], 15
+
+
+void load(S* s) {
+ s->a = -4;
+ s->b = 42;
+ s->c = -12345;
+}
+
+// field 'a'
+// CIR: cir.func dso_local @load
+// CIR: %[[PTR0:.*]] = cir.alloca !cir.ptr<!rec_S>, !cir.ptr<!cir.ptr<!rec_S>>, ["s", init] {alignment = 8 : i64} loc(#loc35)
+// CIR: %[[CONST1:.*]] = cir.const #cir.int<4> : !s32i
+// CIR: %[[MIN1:.*]] = cir.unary(minus, %[[CONST1]]) nsw : !s32i, !s32i
+// CIR: %[[VAL0:.*]] = cir.load align(8) %[[PTR0]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
+// CIR: %[[GET0:.*]] = cir.get_member %[[VAL0]][0] {name = "a"} : !cir.ptr<!rec_S> -> !cir.ptr<!u32i>
+// CIR: %[[SET0:.*]] = cir.set_bitfield(#bfi_a, %[[GET0]] : !cir.ptr<!u32i>, %[[MIN1]] : !s32i) -> !s32i
+
+// LLVM: define dso_local void @load
+// LLVM: %[[PTR0:.*]] = load ptr
+// LLVM: %[[GET0:.*]] = getelementptr %struct.S, ptr %[[PTR0]], i32 0, i32 0
+// LLVM: %[[VAL0:.*]] = load i32, ptr %[[GET0]], align 4
+// LLVM: %[[AND0:.*]] = and i32 %[[VAL0]], 268435455
+// LLVM: %[[OR0:.*]] = or i32 %[[AND0]], -1073741824
+// LLVM: store i32 %[[OR0]], ptr %[[GET0]]
+
+// OGCG: define dso_local void @load
+// OGCG: %[[PTR0:.*]] = load ptr
+// OGCG: %[[VAL0:.*]] = load i32, ptr %[[PTR0]]
+// OGCG: %[[AND0:.*]] = and i32 %[[VAL0]], 268435455
+// OGCG: %[[OR0:.*]] = or i32 %[[AND0]], -1073741824
+// OGCG: store i32 %[[OR0]], ptr %[[PTR0]]
+
+// field 'b'
+// CIR: %[[CONST2:.*]] = cir.const #cir.int<42> : !s32i
+// CIR: %[[VAL1:.*]] = cir.load align(8) %[[PTR0]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
+// CIR: %[[GET1:.*]] = cir.get_member %[[VAL1]][0] {name = "b"} : !cir.ptr<!rec_S> -> !cir.ptr<!u32i>
+// CIR: %[[SET1:.*]] = cir.set_bitfield(#bfi_b, %[[GET1]] : !cir.ptr<!u32i>, %[[CONST2]] : !s32i) -> !s32i
+
+// LLVM: %[[PTR1:.*]] = load ptr
+// LLVM: %[[GET1:.*]] = getelementptr %struct.S, ptr %[[PTR1]], i32 0, i32 0
+// LLVM: %[[VAL1:.*]] = load i32, ptr %[[GET1]], align 4
+// LLVM: %[[AND1:.*]] = and i32 %[[VAL1]], -268304385
+// LLVM: %[[OR1:.*]] = or i32 %[[AND1]], 5505024
+// LLVM: store i32 %[[OR1]], ptr %[[GET1]]
+
+// OGCG: %[[PTR1:.*]] = load ptr
+// OGCG: %[[VAL1:.*]] = load i32, ptr %[[PTR1]]
+// OGCG: %[[AND1:.*]] = and i32 %[[VAL1]], -268304385
+// OGCG: %[[OR1:.*]] = or i32 %[[AND1]], 5505024
+// OGCG: store i32 %[[OR1]], ptr %[[PTR1]]
+
+// field 'c'
+// CIR: %[[CONST3:.*]] = cir.const #cir.int<12345> : !s32i
+// CIR: %[[MIN2:.*]] = cir.unary(minus, %[[CONST3]]) nsw : !s32i, !s32i
+// CIR: %[[VAL2:.*]] = cir.load align(8) %[[PTR0]] : !cir.ptr<!cir.ptr<!rec_S>>, !cir.ptr<!rec_S>
+// CIR: %[[GET2:.*]] = cir.get_member %[[VAL2]][0] {name = "c"} : !cir.ptr<!rec_S> -> !cir.ptr<!u32i>
+// CIR: %[[SET2:.*]] = cir.set_bitfield(#bfi_c, %[[GET2]] : !cir.ptr<!u32i>, %[[MIN2]] : !s32i) -> !s32i
+
+// LLVM: %[[PTR2:.*]] = load ptr
+// LLVM: %[[GET2:.*]] = getelementptr %struct.S, ptr %[[PTR2]], i32 0, i32 0
+// LLVM: %[[VAL2:.*]] = load i32, ptr %[[GET2]], align 4
+// LLVM: %[[AND2:.*]] = and i32 %[[VAL2]], -131072
+// LLVM: %[[OR2:.*]] = or i32 %[[AND2]], 118727
+// LLVM: store i32 %[[OR2]], ptr %[[GET2]]
+
+// OGCG: %[[PTR2:.*]] = load ptr
+// OGCG: %[[VAL2:.*]] = load i32, ptr %[[PTR2]]
+// OGCG: %[[AND2:.*]] = and i32 %[[VAL2]], -131072
+// OGCG: %[[OR2:.*]] = or i32 %[[AND2]], 118727
+// OGCG: store i32 %[[OR2]], ptr %[[PTR2]]
|
erichkeane
reviewed
Jul 9, 2025
| s->a = -4; | ||
| s->b = 42; | ||
| s->c = -12345; | ||
| } |
There was a problem hiding this comment.
Could you do a s->a = s->c; sorta thing in one of the tests?
| 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 |
There was a problem hiding this comment.
Do you have any tests below for a 'volatile' load'? Also the store should be volatile too, right? Should we mention that more elequently?
There was a problem hiding this comment.
Perhaps adding an example would also be helpful
| @@ -224,6 +224,10 @@ void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst, | |||
| return; | |||
| } | |||
|
|
|||
| assert(dst.isBitField() && "NIY LValue type"); | |||
There was a problem hiding this comment.
Suggested change
| assert(dst.isBitField() && "NIY LValue type"); | |
| assert(dst.isBitField() && "NYI LValue type"); |
Also, why is this an assert here for an NYI? This should use the errorNYI stuff instead, perhaps just put htis in a isBitField branch?
| mlir::Type resLTy = convertTypeForMem(dst.getType()); | ||
| Address ptr = dst.getBitFieldAddress(); | ||
|
|
||
| const bool useVolatile = false; |
There was a problem hiding this comment.
Can weput the MissingFeatures assert right next to this?
| cgm.errorNYI(e->getSourceRange(), "bitfields"); | ||
| return {}; | ||
| emitStoreThroughBitfieldLValue(rv, lv); | ||
| } else { |
There was a problem hiding this comment.
don't use curleys on single-line if/else.
| %2 = cir.load %0 : !cir.ptr<!cir.ptr<!record_type>>, !cir.ptr<!record_type> | ||
| %3 = cir.get_member %2[1] {name = "e"} : !cir.ptr<!record_type> | ||
| -> !cir.ptr<!u16i> | ||
| %4 = cir.set_bitfield(#bfi_e, %3 : !cir.ptr<!u16i>, %1 : !s32i) -> !s32i |
There was a problem hiding this comment.
Perhaps we should introduce this one without the parens for args? See https://llvm.github.io/clangir/Dialect/cir-style-guide.html
| 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 |
There was a problem hiding this comment.
Perhaps adding an example would also be helpful
|
|
||
| unsigned storageSize = 0; | ||
|
|
||
| mlir::IntegerType intType = |
There was a problem hiding this comment.
This pattern happened in a recent PR, is it worth having a shared helper for getting this integer type?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR upstreams the
set_bitfieldoperation used to assign values to bitfield members in C and C++ struct types.Handling of AAPCS-specific volatile bitfield semantics will be addressed in a future PR.