commit a5649e0b9c21f6f3e66ace447ee0048f9d315f7f
parent 8360cf077113eea4d157fe6a9c1f229549a1d01e
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Tue, 2 Jun 2026 08:16:02 -0700
cg: remove bitfield_load/store from CgTarget; impls translate (Track 3b follow-up)
The bit-field op was duplicated on both the semantic CgTarget vtable and the
physical NativeTarget vtable (a Principle-1 two-representations smell). Collapse
it to NativeTarget: a bit-field load/store now rides the generic CgTarget
load/store carrying a bit-field MemAccess (new bf_offset/bf_width/bf_signed
rider; bf_width!=0 marks it). The two CgTarget impls translate inside their
load/store: NativeDirectTarget -> NativeTarget bitfield_load/store; IrRecorder
-> CG_IR_BITFIELD_LOAD/STORE. NativeTarget hooks, the recorder+opt IR opcodes,
pass_native_emit, c_target and wasm are unchanged. api_bitfield_access ->
api_mem_for_bitfield; bf_from_mem reconstructs the descriptor in each impl.
bitfield parse corpus 128/0; test-parse 3784/0; toy 1355/0; cg-api 173/0;
bootstrap byte-identical at -O0 and -O1; bootstrapped self-built toy 1355/0.
Diffstat:
7 files changed, 90 insertions(+), 49 deletions(-)
diff --git a/src/arch/check_target.c b/src/arch/check_target.c
@@ -186,14 +186,6 @@ static void check_aggregate(CgTarget* t, Operand dst, Operand src,
(void)a;
}
-static void check_bitfield(CgTarget* t, Operand a, Operand b,
- BitFieldAccess bf) {
- (void)t;
- (void)a;
- (void)b;
- (void)bf;
-}
-
static void check_binop(CgTarget* t, BinOp op, Operand dst, Operand a,
Operand b) {
(void)t;
@@ -395,8 +387,6 @@ static CgTarget* check_backend_make(Compiler* c, ObjBuilder* o,
t->tls_addr_of = check_tls_addr_of;
t->copy_bytes = check_aggregate;
t->set_bytes = check_aggregate;
- t->bitfield_load = check_bitfield;
- t->bitfield_store = check_bitfield;
t->binop = check_binop;
t->unop = check_unop;
t->cmp = check_cmp;
diff --git a/src/cg/cgtarget.h b/src/cg/cgtarget.h
@@ -218,10 +218,20 @@ typedef struct AliasRoot {
typedef struct MemAccess {
CfreeCgTypeId type; /* codegen object type accessed */
- u32 size; /* ABI byte size of this access */
+ u32 size; /* ABI byte size of this access (storage-unit size for a
+ * bit-field) */
u32 align; /* known byte alignment; 0 means unknown */
u16 flags; /* MemFlag */
u16 addr_space;
+ /* Bit-field rider: when bf_width != 0 this access is a bit-field, so `load`
+ * extracts (shift+mask+extend) and `store` inserts (read-modify-write) within
+ * the storage unit described by {type,size}. The CgTarget impls translate this
+ * to the physical NativeTarget bitfield_load/store (or the recorder IR op);
+ * the semantic CgTarget no longer carries a separate bit-field method. */
+ u16 bf_offset; /* target-endian bit offset within the storage unit */
+ u16 bf_width; /* 0 => not a bit-field access */
+ u8 bf_signed; /* signed extraction on load */
+ u8 bf_pad[3];
AliasRoot alias;
} MemAccess;
@@ -249,6 +259,22 @@ typedef struct BitFieldAccess {
u8 pad[3];
} BitFieldAccess;
+/* Reconstruct the BitFieldAccess a CgTarget impl needs from the bit-field
+ * MemAccess that rides the generic load/store (bf_width != 0). The storage unit
+ * is {m.type, m.size}; the bit geometry is the bf_* rider. */
+static inline BitFieldAccess bf_from_mem(MemAccess m) {
+ BitFieldAccess bf = {0};
+ bf.field_type = m.type;
+ bf.storage = m;
+ bf.storage.bf_offset = 0;
+ bf.storage.bf_width = 0;
+ bf.storage.bf_signed = 0;
+ bf.bit_offset = m.bf_offset;
+ bf.bit_width = m.bf_width;
+ bf.signed_ = m.bf_signed;
+ return bf;
+}
+
typedef struct Operand {
u8 kind;
u8 pad[3];
@@ -575,10 +601,10 @@ struct CgTarget {
AggregateAccess);
void (*set_bytes)(CgTarget*, Operand dst_addr, Operand byte_value,
AggregateAccess);
- void (*bitfield_load)(CgTarget*, Operand dst /*LOCAL*/, Operand record_addr,
- BitFieldAccess);
- void (*bitfield_store)(CgTarget*, Operand record_addr,
- Operand src /*LOCAL|IMM*/, BitFieldAccess);
+ /* Bit-fields are not a separate CgTarget method: a bit-field load/store rides
+ * the generic `load`/`store` above with a bit-field MemAccess (bf_width != 0).
+ * Each CgTarget impl translates it (NativeDirectTarget -> NativeTarget's
+ * bitfield_load/store; IrRecorder -> CG_IR_BITFIELD_LOAD/STORE). */
/* ---- arithmetic, compare, convert ----
* binop/unop/cmp accept OPK_LOCAL or OPK_IMM in source operand positions
diff --git a/src/cg/internal.h b/src/cg/internal.h
@@ -403,8 +403,8 @@ int api_sv_op_is(const ApiSValue* sv, OpKind kind);
int api_sv_op_is_local_or_imm(const ApiSValue* sv);
int api_is_lvalue_sv(const ApiSValue* sv);
int api_sv_is_bitfield(const ApiSValue* sv);
-BitFieldAccess api_bitfield_access(CfreeCg* g, const ApiSValue* sv,
- const Operand* storage, CfreeCgTypeId field_ty);
+MemAccess api_mem_for_bitfield(CfreeCg* g, const ApiSValue* sv,
+ const Operand* storage, CfreeCgTypeId field_ty);
void api_stack_grow(CfreeCg* g, u32 want);
void api_push(CfreeCg* g, ApiSValue v);
ApiSValue api_pop(CfreeCg* g);
diff --git a/src/cg/ir_recorder.c b/src/cg/ir_recorder.c
@@ -290,15 +290,32 @@ static void rec_copy(CgTarget* t, Operand dst, Operand src) {
set_ops(rec_of(t), in, ops, 2);
}
+/* Bit-fields ride the generic load/store (mem.bf_width != 0); this impl
+ * translates them to the dedicated CG_IR_BITFIELD_LOAD/STORE opcodes. */
+static void rec_bitfield_load(CgTarget* t, Operand dst, Operand record_addr,
+ BitFieldAccess access);
+static void rec_bitfield_store(CgTarget* t, Operand record_addr, Operand src,
+ BitFieldAccess access);
+
static void rec_load(CgTarget* t, Operand dst, Operand addr, MemAccess mem) {
- CgIrInst* in = emit(rec_of(t), CG_IR_LOAD);
+ CgIrInst* in;
+ if (mem.bf_width != 0) {
+ rec_bitfield_load(t, dst, addr, bf_from_mem(mem));
+ return;
+ }
+ in = emit(rec_of(t), CG_IR_LOAD);
Operand ops[2] = {dst, addr};
set_ops(rec_of(t), in, ops, 2);
in->extra.mem = mem;
}
static void rec_store(CgTarget* t, Operand addr, Operand src, MemAccess mem) {
- CgIrInst* in = emit(rec_of(t), CG_IR_STORE);
+ CgIrInst* in;
+ if (mem.bf_width != 0) {
+ rec_bitfield_store(t, addr, src, bf_from_mem(mem));
+ return;
+ }
+ in = emit(rec_of(t), CG_IR_STORE);
Operand ops[2] = {addr, src};
set_ops(rec_of(t), in, ops, 2);
in->extra.mem = mem;
@@ -634,8 +651,6 @@ CgTarget* cg_ir_recorder_new(Compiler* c, ObjBuilder* obj,
r->base.tls_addr_of = rec_tls_addr_of;
r->base.copy_bytes = rec_copy_bytes;
r->base.set_bytes = rec_set_bytes;
- r->base.bitfield_load = rec_bitfield_load;
- r->base.bitfield_store = rec_bitfield_store;
r->base.binop = rec_binop;
r->base.unop = rec_unop;
r->base.cmp = rec_cmp;
diff --git a/src/cg/memory.c b/src/cg/memory.c
@@ -255,14 +255,13 @@ void cfree_cg_load(CfreeCg* g, CfreeCgMemAccess access) {
api_local_const_clear(api_local_from_handle(g, base.source_local));
}
+ dst_r = api_alloc_temp_local(g, access_ty);
+ dst = api_op_local(dst_r, access_ty);
if (is_bitfield) {
- BitFieldAccess bf = api_bitfield_access(g, &base, &mem_op, access_ty);
- CGLocal rr = api_alloc_temp_local(g, access_ty);
- dst = api_op_local(rr, access_ty);
- T->bitfield_load(T, dst, mem_op, bf);
+ /* A bit-field load rides the generic `load` with a bit-field MemAccess; the
+ * CgTarget impl extracts/extends within the storage unit. */
+ T->load(T, dst, mem_op, api_mem_for_bitfield(g, &base, &mem_op, access_ty));
} else {
- dst_r = api_alloc_temp_local(g, access_ty);
- dst = api_op_local(dst_r, access_ty);
T->load(T, dst, mem_op, api_mem_from_access(g, &mem_op, access));
}
@@ -483,8 +482,9 @@ void cfree_cg_store(CfreeCg* g, CfreeCgMemAccess access) {
}
if (is_bitfield) {
- BitFieldAccess bf = api_bitfield_access(g, &base, &mem_op, access_ty);
- T->bitfield_store(T, mem_op, src, bf);
+ /* A bit-field store rides the generic `store` with a bit-field MemAccess;
+ * the CgTarget impl does the read-modify-write insert. */
+ T->store(T, mem_op, src, api_mem_for_bitfield(g, &base, &mem_op, access_ty));
} else {
T->store(T, mem_op, src, api_mem_from_access(g, &mem_op, access));
}
diff --git a/src/cg/native_direct_target.c b/src/cg/native_direct_target.c
@@ -1231,11 +1231,22 @@ static void nd_copy(CgTarget* t, Operand dst, Operand src) {
nd_release_materialized(d, val);
}
+/* Bit-fields ride the generic load/store (mem.bf_width != 0); this impl
+ * translates them to the physical NativeTarget bitfield_load/store below. */
+static void nd_bitfield_load(CgTarget* t, Operand dst, Operand record_addr,
+ BitFieldAccess access);
+static void nd_bitfield_store(CgTarget* t, Operand record_addr, Operand src,
+ BitFieldAccess access);
+
static void nd_load(CgTarget* t, Operand dst, Operand addr, MemAccess mem) {
NativeDirectTarget* d = nd_of(t);
NdAddrTemps temps;
- u64 size =
- mem.size ? mem.size : (mem.type ? cg_type_size(t->c, mem.type) : 0);
+ u64 size;
+ if (mem.bf_width != 0) {
+ nd_bitfield_load(t, dst, addr, bf_from_mem(mem));
+ return;
+ }
+ size = mem.size ? mem.size : (mem.type ? cg_type_size(t->c, mem.type) : 0);
/* No value-cache flush: only escaped (address-taken / memory-required) locals
* can be aliased through a pointer, and those are never cached. A volatile
* access may observe memory and needs the cache made authoritative first. */
@@ -1272,8 +1283,12 @@ static void nd_load(CgTarget* t, Operand dst, Operand addr, MemAccess mem) {
static void nd_store(CgTarget* t, Operand addr, Operand src, MemAccess mem) {
NativeDirectTarget* d = nd_of(t);
NdAddrTemps temps;
- u64 size =
- mem.size ? mem.size : (mem.type ? cg_type_size(t->c, mem.type) : 0);
+ u64 size;
+ if (mem.bf_width != 0) {
+ nd_bitfield_store(t, addr, src, bf_from_mem(mem));
+ return;
+ }
+ size = mem.size ? mem.size : (mem.type ? cg_type_size(t->c, mem.type) : 0);
/* No value-cache flush (see nd_load): a store through a pointer cannot alias a
* cached non-escaped local. The store target is foreign memory, so there is no
* dst local entry to invalidate; SRC is read via nd_materialize_operand. */
@@ -1785,8 +1800,6 @@ CgTarget* native_direct_target_new(Compiler* c, ObjBuilder* obj,
d->base.tls_addr_of = nd_tls_addr_of;
d->base.copy_bytes = nd_copy_bytes;
d->base.set_bytes = nd_set_bytes;
- d->base.bitfield_load = nd_bitfield_load;
- d->base.bitfield_store = nd_bitfield_store;
d->base.binop = nd_binop;
d->base.unop = nd_unop;
d->base.cmp = nd_cmp;
diff --git a/src/cg/value.c b/src/cg/value.c
@@ -152,20 +152,17 @@ int api_sv_is_bitfield(const ApiSValue* sv) {
* geometry the place carries plus the storage operand it addresses. The storage
* MemAccess is derived from the operand + field type, the way the old
* bf_from_access did, so the place need only carry the bit geometry. */
-BitFieldAccess api_bitfield_access(CfreeCg* g, const ApiSValue* sv,
- const Operand* storage,
- CfreeCgTypeId field_ty) {
- BitFieldAccess bf;
- memset(&bf, 0, sizeof bf);
- bf.field_type = field_ty;
- bf.storage = api_mem_for_lvalue(g, storage, field_ty);
- if (sv->bitfield.bit_storage_size)
- bf.storage.size = sv->bitfield.bit_storage_size;
- bf.storage_offset = 0;
- bf.bit_offset = sv->bitfield.bit_offset;
- bf.bit_width = sv->bitfield.bit_width;
- bf.signed_ = sv->bitfield.bit_signed ? 1 : 0;
- return bf;
+/* Build the bit-field MemAccess that rides the generic load/store: the storage
+ * unit ({type,size}) plus the bf_* bit geometry from the place's descriptor. The
+ * CgTarget impl reconstructs a BitFieldAccess via bf_from_mem. */
+MemAccess api_mem_for_bitfield(CfreeCg* g, const ApiSValue* sv,
+ const Operand* storage, CfreeCgTypeId field_ty) {
+ MemAccess m = api_mem_for_lvalue(g, storage, field_ty);
+ if (sv->bitfield.bit_storage_size) m.size = sv->bitfield.bit_storage_size;
+ m.bf_offset = sv->bitfield.bit_offset;
+ m.bf_width = sv->bitfield.bit_width;
+ m.bf_signed = sv->bitfield.bit_signed ? 1 : 0;
+ return m;
}
void api_stack_grow(CfreeCg* g, u32 want) {
