Conversation
…le object A copy out of a volatile aggregate lost its reads. `struct S t = *p;` through a `volatile struct S *p` moves integer chunks whose types say nothing about the qualifier, so the loads went unmarked and, with `t` unused, DCE deleted every one of them from -O1 up on both targets. The linearizer's block copy and block zero now take the volatility of each end (`BlockVolatility`, from `contains_volatile`) and mark every chunk; a volatile fill within the inline limit is emitted as marked stores rather than a `Memset` that memexpand would expand unmarked. The same family, fixed by the same rule: - A member of an anonymous `volatile` structure was not volatile. `find_member` walked into the anonymous member without collecting its qualifiers; `MemberInfo` now carries what it passed through, applied by one `TypeTable::subobject_type` that member access uses too. The anonymous-aggregate predicate written out four times is one `TypeTable::is_anonymous_aggregate`. - An initializer's stores into a `volatile` object were unmarked, since the walk stores at each member's declared type. The walk names the object it is storing into while the subobject inherits `volatile`, and `mark_volatile_access` -- the one place a marker is decided -- marks every access addressed through it, including a bit-field's carrier load. - `effects` inferred `Pure`/`Const` for a function whose body is a volatile access; a volatile access is `Unknown` now. And the rule itself was written in several places that disagreed: - `loadfwd::forwardable` and `dse::deletable` were one body twice, asking the top-level qualifier where `contains_volatile` is the question. They are one `memloc::is_ordinary_object`. - `LocalVar::is_volatile`/`is_atomic` were flags five linearizer sites each derived from the type, and others passed `false` for types that could be qualified. The flags are gone: `LocalVar::is_ordinary(types)` asks the type, so no constructor can disagree with it, and `add_local` loses two parameters and its `too_many_arguments` allow. - `TypeTable::is_atomic` replaces the spelled-out modifier test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d7bdcd2 made every empty write keep its call -- `printf("")`, `fprintf(fp, "")`, `fputs("", fp)` -- because C17 7.21.2p4 orients a stream on the first output function applied to it, whether or not a byte moves. Seven torture tests assert gcc's opposite choice: their replacement library aborts if an empty write reaches it at -O1 and up. The harness still expected them to pass, so every torture run since has reported seven regressions per target that are a decision, not a defect. They are now named in OUT_OF_SCOPE_GCC_BEHAVIOUR with the reason, dropped from both baselines, and recorded beside the other gcc-specific behaviours in DECISIONS.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lways
An `asm goto` label with a phi read the wrong value, at every level. The
label is reached by the jump and by the fallthrough, so phi elimination put
the copy for the jump's edge at the end of the `asm` block -- after the `asm`,
which the jump leaves before reaching. The copy never ran, and
int r = x + 1; if (x > 5) r = 7;
asm goto("jmp %l0" :::: L); r = 2; L: return r;
returned x + 1 for x = 10. The same placement is the textbook lost-copy
miscompile on any critical edge once a pass rewrites a use to a phi's target.
`lower` now splits every critical edge into a block with phis before it
eliminates them (`ir/cfg.rs`, which records the contract with CFG
simplification: splitting happens once, there, and nothing merges blocks
after it). An edge out of a computed `goto` cannot be split; a phi reached
along one takes its operands in a temporary of its own and becomes the copy
out of it at the head of its block (Sreedhar's method I).
The CFG itself had no single owner:
- `children`/`parents` are a cache of what the instructions say, and edges
were dropped from one list and not the other: `dce`'s branch fold trimmed
`children` only and left the phi operand and its `PhiSource` behind. Every
edge edit now goes through `Function::add_edge`/`remove_edge`, which keep
both lists, the phi operands and the `PhiSource`s together; `dce` folds a
branch through `propagate::retarget_terminator`, and unreachable-block
removal and `ifconv` remove edges the same way. `retain_edges` and
`remove_phi_predecessor` are gone.
- Three passes rebuilt predecessors from `children` because `parents` could
not be trusted, and "successors plus the `asm goto` labels" was computed six
times. `children` includes the labels, so `dominate`, `loadfwd`, `dse`,
`vrp` and `sccp` read the cache (`successor_map`/`predecessor_map`), and
`propagate::terminator_targets` became `Instruction::control_targets`.
The comment claiming `dominate` reads `parents` was false and is gone.
- `Instruction::kill` reset four fields of twenty-five, so a killed branch
still named its targets and several passes rewrote in place or skipped
`Nop` to route around it. It resets the whole instruction now.
`validate` checked six invariants, none about the CFG, and ran only under
`debug_assert!` -- which `cargo test --release`, the torture harness and a
released `c17` all skip. It now also checks that every block ends in one
terminator, that `children` is exactly what the instructions name, that
`parents` is its inverse, that the block index is current (I8), and that
phis and `PhiSource`s agree with the edges (I9). It runs always, after
linearization, after target mapping, after optimization and after lowering,
and stops the compiler with an internal-error report naming the stage.
Running it found one more: `longjmp` and `__builtin_unreachable` ended their
block and left the rest of the statement in it, after the terminator. They
now go through one `emit_no_return`, as the `Unreachable` after a `noreturn`
call already did, and continue in a fresh unreachable block.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er walks
Qualifiers leaked onto values. An assignment, compound assignment, `++`/`--`,
a cast and a call were typed with the operand's qualified type, where C17
gives each the unqualified one (6.5.16p3, 6.5.4p5, 6.7.6.3p4): `v = 1` for a
`volatile int v` was a `volatile int`, which `__typeof__` could see. The five
modification forms share one `modification_result_type`, the cast and the
call take `unqualified`. The conditional merged two pointer arms by taking
the first one's type, so `c ? p : cp` lost the `const` that `c ? cp : p`
kept; it now forms the pointer to the composite type qualified by both
pointees, or to qualified `void` (6.5.15p6), and the unreachable
floating-point block after the arithmetic arm is gone.
The initializer walks had rules written twice that disagreed:
- A union's brace-elided initializer counted its first *named* member, while
the walk that places the values fills the first member, anonymous or not:
`union U { struct { int a, b; }; long q; } u[] = {1, 2, 3, 4}` came out as
four elements of one value each. `StructMember::is_initializable` is now the
one test, in both.
- The array initializer cursor (where an element lands, how far a GNU range
moves it) and the brace-elision span were each written in the parser, to
size `int a[] = {...}`, and again in the linearizer, to place the values --
one copy was commented as "a second, independent copy". Both live in
`parse::ast` as `array_slot` and `brace_elision_span`.
- `offsetof` and the member step of a static address were walked in
`constexpr` and again in the linearizer; `constexpr::offset_of` and
`member_at` are the one walk, which the linearizer (a `ConstEnv`) calls.
Designated overrides reaching inside an earlier initializer:
- A string literal is one initializer for its whole array, so
`{ .s = "abcd", .s[1] = 'z' }` on a static object had nothing to replace the
element in and dropped the literal: `s` came out "\0z". The literal is taken
apart into its elements (`Initializer::string_as_array`) first.
- A union initialized from a whole value, `{ .u = v, .u.s.b = 9 }`, was reset
while the same shape through a struct kept `v`'s other bytes. Which member
`v` holds is known only at run time; its bytes are recorded as a value
(`Held::Value`), which agrees with whatever member a later designator
names. gcc discards `v` in both cases; recorded in DECISIONS.md.
`branch_on` returned without a branch when there was no current block,
leaving both targets one predecessor short with nothing to say so; it starts
an unreachable block like every other terminator, and the wide-switch case
chain, which had the same early return, goes through it.
Two shapes a deleted branch once miscompiled -- an out-of-order designated
initializer of an eight-byte struct, and the zero tail of `char s[13] = "ab"`
in a dirty frame -- were probed on both targets at -O0 and -O2, are already
right, and are pinned by a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…FA and x87 returns Whether an aggregate comes back in registers was decided twice, and the two ends of a call disagreed. The caller asked "a struct or union wider than one register, not through the hidden pointer"; the callee asked the same with a sixteen-byte cap. AAPCS64 returns a homogeneous floating-point aggregate in d0-d3 at any size, so a three- or four-`double` HFA was a register aggregate to its caller and an ordinary return to its callee, whose `Ret` handed back an address under no ABI classification. The inliner copies an address-returned aggregate into the call's result local from exactly that classification, so without it every HFA and x87 return had to be refused inlining (`Function::ret_is_address`). One `Linearizer::returns_reg_aggregate` now answers for both ends, and `emit_reg_aggregate_return` puts the classification on every such `Ret`. `ret_is_address` is back to meaning a `_Complex` return only, and the test that pinned the refusal now asserts the splice and runs the result on both targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… it was Every inlining decision for a caller was made before any splice, against the caller's size measured once; the refresh after each splice came too late for any decision to see. So any number of small callees each passed the growth and stack caps against the same unchanged size: thirty calls to a ten-instruction leaf all went in where the limit has room for twenty-six. Each accepted call site now adds its callee to the size the next decision sees. The growth limit was also proportional to the caller's *current* size, and the pass iterates, so every iteration granted the whole allowance again. It is measured against the caller as it was before the pass began, as gcc measures it. And the size itself counted raw instructions -- `Nop`, `Entry`, phis and their sources, and the `Copy`s and `SetVal`s promotion leaves by the dozen, close to half of any count, and growing as passes kill instructions. One `insn_cost` now decides what counts, for a callee and a caller alike. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ncluded For about a hundred opcodes `typ`/`size` describe the result; for the sixteen comparisons and the two population counts they described the *operand*. Nothing enforced it, and every consumer had to know: two folds that copied a comparison's `typ` onto the constant replacing it were separately patched miscompiles (a float compare's folded 0/1 returned in an SSE register), a `result_type_of` existed only to undo the convention, `cmp_operand_width` guessed between `size` and `src_size`, and one construction site -- the `_Bool` conversion -- already did the opposite of the others. Now `typ`/`size` are the result for every opcode, and an opcode that reads another type than it produces -- the conversions, the comparisons, the population counts (`Opcode::reads_another_type`) -- records its operands in `src_typ`/`src_size`, read through `Instruction::operand_type` and `operand_width`. Comparisons are built by one `Instruction::compare` (and `Builder::compare`, `Linearizer::emit_compare`), which takes the operand and result types separately; `Instruction::binop` refuses a comparison outright, in release builds too, and the verifier (I10) rejects one recorded without its operands. `result_type_of`, `cmp_operand_width`, the linearizer's `emit_fcmp` duplicate of the compare emitter, and the `TypeTable` that `sccp` and `vrp` carried only to retype folded comparisons are gone. Every reader that meant the operand now asks for it: the 128-bit comparison split in `mapping`, the `_Float16` and binary128 comparison lowerings, x87 detection, the integer and floating compare emitters on both targets, `instcombine`'s float-comparison outcomes, and the `long double`/`__float128` pseudo classification. The hand-written lists of the sixteen opcodes that only *classify* -- in the two back ends' dispatch, `instcombine`, `sccp`, `ifconv`, register-class selection and the int128 classifier -- use `Opcode::is_int_comparison`, `is_float_comparison` and `is_comparison`; `vrp` recognized an integer comparison by testing whether its printed name started with "fcmp". The verifier's new check found comparisons with no operand width at all: a function designator compared -- `if (weak_fn)`, `f == g` -- typed as the function, whose width is 0, which both back ends raised to 32 bits, so the low half of the address was all that was compared. The linearizer's one comparison constructor (`compare_insn`) now derives the operand width itself, and compares an array or a function as the pointer it decays to. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`Instruction` was 248 bytes, and every one of them paid for the fields only calls, switches, `asm` and atomics use: the callee and its argument types, the variadic and noreturn markers, the library-function tag, the indirect target, the switch cases and default, the `asm` operands, the ABI record and the memory order. Those thirteen now live in one `InsnExtra` box that every other instruction leaves empty, read through `Instruction::extra()` (a shared empty record when absent) and set through `extra_mut()`. `Instruction` is 128 bytes, pinned by a test. Compiling CPython's ceval.c at -O2 takes 24% less peak memory (670 MB to 510 MB) and slightly less time, and the assembly is byte-identical for ceval.c, dictobject.c and unicodeobject.c at -O0 and -O2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts at compile time
Promotion out of memory gives every read of a local its own `Copy`, and phi
elimination, inlining and folding add more; nothing removed them, so each
became a register-to-register move. On five ordinary functions at -O2 the IR
was 126 instructions of which 35 were copies and 28 `Nop`s, x86-64 emitted
90 moves in 196 instructions and aarch64 69 in 157 -- swapping `a + b`'s
operands through a temporary.
`ir/copyprop.rs` rewrites each use of a copy's result to its source and
leaves the unused copy for `dce`. A copy is followed only when it is a
no-op: the same width as its source's definition and the same register
class (a narrowing copy is not -- the back ends extend it, and compare at no
less than 32 bits), or a constant that reads the same at the copy's width.
It runs in the optimizer's loop; the critical-edge splitting done before
phi elimination is what makes forwarding into phi operands safe. Each
iteration also drops the `Nop`s every pass had to skip. Same sample after:
63 IR instructions with no copies and no `Nop`s, 60 moves in 166 on x86-64,
38 in 126 on aarch64, and no swap.
Forwarding let arguments flow straight into calls, which exposed the back
ends inferring an argument's class -- x87 `long double`, `__float128`,
`__int128`, floating point -- from the instructions using it rather than from
its parameter type: a `__float128` argument passed directly to a call got
an 8-byte slot it was stored into with `movsd` and reloaded from with
`movups`. Every classification now seeds arguments from
`arch::regalloc::arg_pseudo_types`.
Two more x86-64 emitters had never seen an operand outside a register. The
floating and x87 `va_arg` paths used R11 as scratch while R11 held the
`va_list` pointer, and stored through the clobbered value; `emit_va_arg` now
always holds the pointer in R11 and neither helper touches it. And the x87
integer conversion stored a constant past 32 bits straight to memory, which
nothing encodes. "Does this immediate fit the instruction" was written out at
three call sites and missing at the fourth; it is `imm_operand`, with
`gp_operand_via` and `store_imm` materializing through a named scratch when
it does not.
A narrow copy of a constant that does survive is materialized already
extended on both targets, through the shared `constfold::at_width` (now
reachable from `arch/`), and a narrow register copy uses `movzx`/`movsx`
rather than a shift pair: `char buf[16] = {0}` no longer spends
`shll $24; sarl $24` on its zero byte, at -O2 or -O0.
Two aarch64 assembly-shape tests named a specific scratch register; they
follow the register that reaches the argument now.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ptimizer loop `sccp` and `vrp` each carried their own Wegman-Zadeck solver: the same seeding (`Undef` and `Sym` overdefined, inline-asm outputs unfoldable, address-taken blocks reachable with no edge), the same `asm goto` edge rule, the same three worklists, and two copies of rewriting what the solution proves -- `vrp`'s seed was documented as "copied from `sccp::seed`, decision for decision". Each of those decisions is a way the algorithm turns unsound when one copy drifts. `ir/dataflow.rs` now holds the one copy. A pass supplies a `Lattice` and three answers -- what an instruction produces, what a branch selector is known to be, and what constant a solved pseudo holds -- plus, for `vrp`, widening and the edge facts it derives after a `Cbr`; the solver owns the rest, including the rule that a solve cut short by its step budget changes nothing. `cbr_taken`/`switch_taken` are applied in one place for both marking edges and folding terminators, where each pass had previously written the decision twice. `Opcode::is_int_arith` replaces the two `is_modelled_binop` lists. The `switch_taken` tests move to `propagate`, whose function they test. `MemLoc::byte_extent` is the bits-to-bytes extent that `dse::covers`, `memloc::may_alias` and `loadfwd::byte_index` each computed. The optimizer's fixed-point loop is a table of passes in order, each with the reason for its place, and counts what every pass changed. A function that reaches `MAX_ITERATIONS` still changing used to stop silently; it is now reported -- with the passes still moving -- under `--dump-ir post-opt`. Across gcc.c-torture that is four functions, all in strlen-5.c: each `strlen` there folds only once the `printf` guarding the previous one has been deleted, one per round, because the dead block survives until `dce` at the end of the round. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the store `loadfwd` checks that nothing writes the location on any path from the definition to the load, over the blocks forward-reachable from the definition and backward-reachable from the load. That region took in the definition's own block whenever it sat in a loop, and when the store and the load shared a block in a loop it scanned the whole block. Either way the write a loop makes *after* the load -- `a[0] = i; s += a[0]; a[0] = 5;` -- was counted as lying between the two, and the load stayed. It does not lie between them: a block is entered only at its top, so any path that comes back to the definition's block runs the definition again and starts over. The definition's block is now a barrier neither walk passes through, and when the two share a block only the instructions between them are scanned. An inner loop around the load that avoids the store still puts the load's block in the region, and still refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e, and simplify before inlining Nothing merged blocks or threaded empty forwarders: once SCCP and DCE had removed what made a block more than a branch, the block stayed, and every one became a jump in the output. `Function::simplify_cfg` in `ir/cfg.rs` drops unreachable blocks, sends each edge into a block that only branches on straight to its destination, and merges a block into its predecessor when each is the other's only neighbour -- its single-input phis, and the `PhiSource`s feeding them, becoming copies. A forwarder that is the entry, has its address taken, or leads into a block with phis is left alone, as is a cycle of forwarders, which is a loop the program asked for. It runs last in the optimizer's loop; nothing merges blocks after `split_critical_edges`, as the module comment already required. Across gcc.c-torture's 2000* execute tests, -O2 x86-64 output has 512 `jmp`s where it had 1273, and 12% fewer instructions in all. A chain of forwarders is resolved in one walk, every link given the answer, and a block's redirected edges are rewritten together: compile/limits-caselabels, a switch whose 100,000 case labels are one long chain, took two minutes when each link followed the rest. `remove_unreachable_blocks` moves from `dce` to `cfg.rs`, the file that owns every CFG edit; `Instruction::retarget` no longer allocates an `InsnExtra` for an instruction that has none; and renaming a predecessor in a block's `parents` and phis is one `BasicBlock::rename_predecessor` shared by edge splitting and merging. `constglobal` ran once, before the loop, and matched only a load whose operand was the global's symbol itself: `const int *p = &k; return *p;` kept its load. It now resolves the load's address with `AddrMap::resolve`, as every memory pass does, and runs in the loop, where `instcombine` has folded the arithmetic an address is built from. The globals it knows are collected once per module (`KnownGlobals`). Before inlining, every function gets one round of `sccp`, `instcombine`, `copyprop`, `dce` and `simplify_cfg`, so the inliner sizes a callee by the code it will emit. Two tests relied on the old sizing: a recursive-guard fixture whose body reset its accumulator every seventh line (`s ^= s << 0`), so all but three of its statements were dead and the callee was small; and a spill test whose `static` callee is now small enough to inline, kept out of line with `noinline`. `instcombine` has no strength reduction to move into `arch/mapping`. The four strlen-5.c functions still reach the iteration cap: each `strlen` there is guarded by a `puts` of the same array, which may write it -- a stdio call can write a buffer the program handed to `setvbuf` -- until the branch on the previous result folds, one round later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e optimizer emptied
Every x86-64 frame began with sixteen bytes for the x87 scratch, the slot
`x87.rs` stages a value through on its way into or out of the FPU -- though
only an x87 conversion or an x87 inline-asm operand ever does. And the
locals the linearizer created kept their slots however completely the
optimizer forwarded and deleted their accesses: `int a[4] = {1, 2, 3, 4};
return a[2];` folds to `return 3` and still reserved the array.
The allocator now reserves the scratch -- like the x87 control words before
it, in one `reserve_x87_frame` -- only for a function with an instruction
that `x87::uses_x87_scratch`: an integer to `long double` conversion, a
`long double` to integer one, a conversion between `long double` and another
floating type, or an `asm` with a `float` or `double` operand on the x87
stack. Code generation dispatches on the same three conversion predicates,
so the reservation and its users cannot disagree, and the scratch is
addressed through the slot the allocator recorded (`X87Scratch`) rather than
a fixed offset: an unreserved use is an internal error, not a write over the
first local. The x87 asm operand walk, with its tied-constraint resolution,
is one `inline_asm::x87_operands` shared with the allocator's classification.
`mem2reg` runs again after the optimizer's loop, and drops the
`implicit_param_copies` record of a register-passed parameter local it
removes; the prologues on both targets already store nothing for a missing
local.
`zero_stack_frame`'s comment said it ran after the argument spills and must;
it runs before them, and must, or it would wipe them.
Across gcc.c-torture's 2000* execute tests, -O2 x86-64 frames total 2232
bytes where they totalled 6336. `int plus1(int x) { return x + 1; }` has no
frame at all. Two tests leaned on what is gone: one asserted a parameter-only
function's frame was at most sixteen bytes, now none; and the over-aligned
frame probe's `_Alignas(32)` array was folded away entirely, leaving nothing
to realign -- it now escapes, and the test asserts the realignment it
depends on.
Two compile-time quadratics, which made compile/limits-fndefn (100,000
parameters) run out the torture harness's time. `param_type_of_arg` found
the hidden sret pointer by walking the pseudo table, once per argument, from
loops over every argument in `copyprop` and the allocators;
`Function::arg_types` finds it once, and `param_type_of_arg` and
`AbiLowering` both read it: 120 seconds is half of one. And `Module::add_global` found a
tentative definition by walking every global: a `GlobalIndex` catches up
with whatever was appended and checks its answer, taking
compile/limits-externalid from fifteen seconds to under one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/codegen/misc.rs had grown to 570 KB and cross_abi.rs to 147 KB, too large to read or review whole. misc.rs becomes ten files -- optimizer, symbols, types_exprs, asm_attributes, inlining, aggregate_abi, call_args, floating, varargs and int128 -- and cross_abi.rs keeps its aggregate and register-parameter tests, with cross_abi_types and cross_abi_varargs beside it. Every file is now under 100 KB, and every test moved unchanged. Each file imports only what it uses. A name only cfg-gated tests use is spelled in full rather than imported, so no platform sees an unused import. Helpers two files share moved to where both can see them: `asm_probe` gains `section_of` and `host_asm` -- the latter was misc.rs's own `asm_for`, a second function under the name `asm_probe` already exports with another signature -- and `common` gains `compile_and_run_everywhere`, which one test had also written out in full. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… frame Both prologues zeroed the whole locals area so that "a narrow write leaves zero above it" -- a cost on every call that hid a class of defect rather than fixing it. With the zeroing switched off, gcc.c-torture stayed green on both targets and so did the test suite, but a probe that dirties the stack before spilling found the class at once: on aarch64 a comparison's `int` result was spilled with `str w16, [x29, #136]` and tested as a branch condition with `ldr x9, [x29, #136]`, the upper half whatever the frame held. `Cbr` carries no width for its condition, so each back end guessed: aarch64 read sixty-four bits, x86-64 derived a width from a type `Cbr` is never given and fell back to sixty-four, and both `Select`s read sixty-four. The width a value is held at is its defining instruction's size -- the width a spill stores it at -- and `arch::codegen::ValueWidths` records it per function for both targets. Each target's `emit_condition_test` reads a condition at that width, extending a sub-32-bit one on aarch64, and is the one test `Cbr` and `Select` both make. The rule for widening a store into a local was written once per back end and they disagreed: x86-64 widened a 32-bit store into an `int`'s own slot, and into any slot it had no record of, where aarch64 did neither. `SymSlot::store_bits` is the rule both now ask: 32 bits at offset 0 of a slot holding one scalar wider than that, and nothing else. With every value read back at the width it was stored at, the zeroing -- `rep stosq` on x86-64, `stp xzr, xzr` pairs or a loop on aarch64 -- is gone. An uninitialized object has no value in C, and gcc never zeroed one either. The aarch64 tests of the zeroing's own shape become a test that a large frame runs and one that a prologue does not grow with its frame, and the function-designator test now asks that the *address* be compared at 64 bits, not that no 32-bit compare appear: the `int` that comparison yields is rightly tested at 32. CPython's suite passes unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…share a slot Every local held its own frame slot for the whole function. A local is a `Sym`, which no instruction defines, so liveness found each of its uses upward-exposed and carried it to function entry: every local overlapped every other from position 0, and the slot reuse both allocators have could never fire -- they set `reusable = false` for locals, with a comment explaining that the intervals were wrong. The linearizer now marks where control falls out of a block scope with a `LifetimeEnd` for each local it declared (C17 6.2.4p6). The marker names its local out of band, in `InsnExtra::lifetime_of` and never in `src`, so no analysis counts it as a use or an escape; `mem2reg` drops it with its local, the inliner remaps it, DCE keeps it, and validator invariant I11 requires it to name a local of its own function. `arch::regalloc::local_lifetimes` turns the markers into intervals: a local is "may be live" forward from any instruction that mentions it until its `LifetimeEnd`, by a dataflow over the CFG, and its interval is the hull of that region. A jump past a declaration reaches a mention all the same; a path that leaves a scope without its marker carries the object further, the safe direction; and a use after the end is undefined behaviour, so a pointer into the object needs no tracking of its own. A parameter's local, which the prologue writes, is live from entry, and in a function that calls `setjmp` every local spans the whole function. `arch::regalloc::place_locals` then gives each local a slot, sharing one when two agree in size and alignment and their lifetimes never meet. It lays them out in declaration order, a pre-pass ahead of coloring, as they always were: with lifetimes in the coloring loop they were placed by first use, and the small locals of the inline-asm memory operand test moved past a 40 KB array, out of reach of `[x29, #imm]`. The size and alignment of a local's slot is one `local_slot` for both targets, and the unused `addr_taken_syms` placeholder is gone from both. CPython's `_PyEval_EvalFrameDefault` frame is 3240 bytes where it was 5144, and its suite passes. At the default 8 MB stack three recursion-limit tests still overflow, each a segfault deep in a recursion: test_call, test_compile and test_isinstance. The last measurement at 8 MB, some compiler versions ago, had five failing -- test_call, test_descr, test_io, test_isinstance, test_userdict -- and test_compile was not among them; it was not measured immediately before this change. gcc's frame for the same function is 376 bytes, and what remains is spill pressure, not locals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… copies were A bit-field's placement travelled as a five-tuple -- `(byte offset, bit offset, bit width, storage bytes, field type)`, "spelled to match" the two emitters that consumed it -- and those emitters took it as five loose arguments under `#[allow(clippy::too_many_arguments)]`. `types::Bitfield` names the four placement fields; `MemberInfo`, `StructMember` and the initializer walk's `StructFieldVisit` each answer `bitfield()` from their own fields; and `emit_bitfield_load`/`_store` and their byte-wise variants take one. The bytes a bit-field's own bits occupy -- narrower than its access span -- were computed in three places, two of them in the same initializer walk; they are `types::own_bit_bytes`. `Linearizer::offset_address` was the helper for `base + offset`, and six sites built the constant and the `Add` by hand beside it. The IDF walk in `dominate` examined J-edges in two places with the same rule -- once for each block taken off the queue and once for each block of the subtree under it -- threading five pieces of mutable state through a nine-argument function; it is `IdfWalk`, with one `j_edges`. `dse` threads the function and its four analyses as `Facts`, of which `scan_block`, `may_read` and `dead_locals_at_block_end` are methods. Both targets' select emitters take `arch::codegen::SelectOperands`, decoded once from the instruction with the width both targets computed the same way. The `too_many_arguments` allows on these go, as do two that had gone stale on the allocators' `run_chordal_color`s and one that had drifted onto `Module::global_mut`; seven remain, on signatures where a struct would be an argument list by another name. SSA conversion kept its phi table on `BasicBlock` as `phi_map`, where it outlived conversion on every block for nothing; it is the converter's `phis` now, and the `all_phis` list it also kept and never read is gone, as is the empty "dominator tree fields" banner the struct still carried. The SSA tests find a block's phi by its instruction, and one of them no longer asserts nothing when the phi is missing. `emit_two_way` was `emit_diamond` at the type's own width for its one caller, which now says so. `local_slot` records why a local's slot stays at least eight bytes: the back ends move a register-passed value or an aggregate's tail a whole eightbyte at a time. Slots at each object's own size were tried, and broke a three-byte struct, `_Float16` and plain `char` parameters, among others. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ir names, and bring the docs up to date
`ir/facts.rs` had no tests of its own, and `ir/propagate.rs` only the
`switch_taken` ones; each was exercised only through the passes that use it.
`ConstMap` is now tested for following a copy only at its width, reading a
constant at the width and sign asked, refusing an ambiguous fold, and
treating an inline-asm output as no copy; `CmpFacts` for recording a
comparison by its operands' roots and reading it through a copy;
`fold_target_to_const`, `retarget_terminator` and `cbr_taken` for what each
rewrites, keeps and decides. `cbr_taken`'s rationale now names why a branch
condition is at least 32 bits -- the linearizer makes it a comparison's
`int` -- since the back ends test a condition at its own width.
`test_linearize.rs` asked a dump whether an opcode's name appeared in it, so
`ir.contains("br")` was satisfied by every `cbr`. The seventy checks that
name an opcode, or a family such as `div` or `fcmp`, ask the instructions
through `has_op`; those that check a width, an operand or a name in the
dump stay textual.
`ir/README.md` describes the pipeline as it runs -- the pre-inline round,
`constglobal` and `simplify_cfg` in the loop, `mem2reg` after it, the report
of an unfinished loop -- lists `dataflow`, `copyprop`, `cfg`, `dse`, `mem2reg`
and `validate`, documents `lifetime.end`, and stops naming
`LocalVar::is_volatile` and `phi_map`, both gone. `TODO.md` drops what is
done (CFG simplification, copy propagation, the frame zeroing, unreused
local slots), takes LICM and loop canonicalization off in favour of GVN and
global code motion as one project, aims its peephole table at the two
patterns c17 actually emits in bulk, and describes the stack-frame and
R10/R11 items as they stand. `cc/CLAUDE.md` notes that CI runs the suite
single-threaded and that two full runs at once produce false failures.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The changes touch sensitive multi-architecture register allocation, optimization-pass correctness, and type-qualifier semantics where subtle miscompiles cannot be fully ruled out from the diff, warranting expert human review.
Review effort: Balanced
Findings: None
What changed in this PR
This PR is a broad set of fixes and cleanups to the cc C compiler component of posixutils-rs. It refactors bit-field/member placement logic into reusable helpers, correctly propagates _Atomic/qualifier information through anonymous aggregates and subobjects, modernizes the compiler's documentation/backlog to match the current register-allocator and optimizer design, and adds cross-architecture test helpers plus regression tests for store/reload width correctness.
Changes:
- Introduce a
Bitfieldtype plusown_bit_bytes,is_initializable,is_anonymous_aggregate,subobject_type, andis_atomichelpers incc/types.rs, and thread member qualifiers (quals) throughMemberInfo/find_member_recursiveso qualifier inheritance through anonymous structs/unions is handled in one place. - Rewrite the
cc/TODO.mdbacklog to reflect the current design: lifetime-based frame-slot sharing (replacing frame zeroing), R10/R11 scratch reservation, and a consolidated GVN/Global-Code-Motion plan. - Add the
compile_and_run_everywherecross-arch (host + aarch64/qemu, -O0/-O2) test helper and new register-allocation regression tests for read-at-store-width correctness.
| File | Description |
|---|---|
cc/types.rs |
Adds Bitfield/placement helpers, is_initializable/is_anonymous_aggregate/subobject_type/is_atomic, and member-qualifier propagation (MemberInfo.quals) for anonymous aggregates. |
cc/TODO.md |
Updates the compiler backlog to match the current register allocator and optimizer, consolidating stale pass notes into a GVN/GCM plan. |
cc/tests/common/mod.rs |
Adds compile_and_run_everywhere to run a source across -O0/-O2 on host and (when available) aarch64 under qemu. |
cc/tests/codegen/regalloc.rs |
Adds asm-probe imports and a regression test ensuring spilled values are read back at the width they were stored. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`codegen_a_frame_holds_only_what_the_function_uses` compiled its run program with `compile_and_run`, which targets the host, and that program holds an x87 `asm` (`fsqrt`, `"+t"`). On the aarch64 runners -- Linux and macOS -- it cannot assemble (`expected a register at operand 1 -- 'fsqrt'`), so the test failed there while passing on x86-64. Off x86-64 the host now runs the form the test already built for qemu, with the asm replaced by plain C; the assembly-shape assertions name their targets and still run everywhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sential, and state the _Imaginary policy The Conformance section described a retired audit file and how its findings were recorded -- history the git log already holds -- under a heading a reader would take for the compiler's conformance status. It goes, with the Overview bullet that linked to it. The key source files listed thirty-nine rows, nearly every file, which made "key" mean nothing, and two were wrong: `mem2reg.rs` drops unreferenced locals rather than promoting them, and `opt.rs` runs a dozen passes, not two. Fourteen remain, in pipeline order, each where a central data structure, algorithm or contract lives; `ir/README.md` lists every pass. `_Imaginary` is stated as the C17 minimum. Imaginary types belong to Annex G, which binds only an implementation that defines `__STDC_IEC_559_COMPLEX__`; c17 does not, so it owes what remains: `_Imaginary` is a keyword, a use of it is diagnosed (6.7.2p2), and `imaginary`/`_Imaginary_I` stay undefined (POSIX `<complex.h>`). The entry said gcc defines that macro; glibc's `<stdc-predef.h>` does, for gcc. It also notes that the GNU `2i` suffix is supported and gives a `_Complex`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ype for `_Imaginary` was tagged only as a reserved name, so `_Imaginary double x;` was not taken as a declaration: it drew implicit-int and then "'_Imaginary' is a keyword and cannot be used as a name", neither of which names the problem. Imaginary types are Annex G's, binding only an implementation that defines __STDC_IEC_559_COMPLEX__, which c17 does not; without them `_Imaginary` is no permitted type specifier (C17 6.7.2p2) and needs one diagnostic. The keyword is now tagged as a type specifier, and the specifier loop reports "imaginary types are not supported" once, then carries on as if `_Complex` had been written. After a type specifier and before what can only follow a declarator's name, it is that name, as a typedef name would be, so `int _Imaginary;` still reports a keyword misused. In a type-name it is always the specifier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…R/LIR tour and references, and audit ATTR.md ir/README.md: - Opcodes: list the six that were missing (indirectbr, tlsaddr, va_arg_pack_len, constant_p, stacksave, stackrestore), use the names the dump prints (sel, frame_address, return_address, atomic_*), regroup the misfiled float unary ops, and describe fcmp_one as the unordered != it is. - Passes: Lengauer-Tarjan, not Cooper; mem2reg deletes unreferenced locals rather than promoting; add tls, mach_o_dtors and linearize_atomic; separate passes from shared analyses; state the pipeline from linearization to lowering with its validator stages. - Instruction fields: split the inline fields from InsnExtra and list all of both. - New: a tour of HIR vs LIR (cc/arch), the path from one to the other, key data structures, and how to write a new pass; and a references list led by sparse. ir/dominate.rs: the header named the dominator algorithm it replaced. ATTR.md: audited against the source. Corrects ms_abi, pure/const, Mach-O destructors, noreturn and __has_attribute coverage; adds missing attributes, argument diagnostics, and how to add a new attribute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both were given a name check of their own in parse_single_attribute when SUPPORTED_ATTR became the one list, because neither was implemented then. Implementing them emptied those branches but kept the name checks and never added the tag, so __has_attribute went on answering 0 for two attributes the compiler implements. Tag both spellings of each and delete the name checks, so the tag alone decides recognition. A pending mode, vector width or alignment is now taken only from a recognised spelling: `__mode(QI)` was warned about as ignored and applied anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Audited against the source. Deleted: history and status prose, the torture-suite and clang-suite rows, an item C17 itself settles (`cond ? void_call() : 0` is rightly rejected), local CSE (covered by GVN+GCM), and TLS items not chosen. Each remaining item says what is wrong, where, and what done looks like; latent Local-Exec sites become their own item, and a Conformance section records `restrict int x;` being accepted against C17 6.7.3p2. The float constant-folding rule, a settled choice, moves to DECISIONS.md, checked against ir/constfold.rs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uiltin Corrects ffs (a libc call), which bit builtins fold, __builtin_flt_rounds, unreachable, expect, prefetch, complex, classify_type, choose_expr, strnlen's spellings, placeholder-declared builtins' arity, and what each target does with a memory order. Adds how names are recognised, argument checking, where __has_builtin differs from gcc, the lowering of each builtin, the call folds, more not-implemented rows, and an "Adding a Builtin" section for parser-folded builtins, new IR opcodes and library functions, with the tests each needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`__attribute__((fallthrough));` -- the GNU spelling of C23's [[fallthrough]], and what `-Wimplicit-fallthrough` code writes -- was rejected with "declaration declares nothing": `__attribute__` starts a declaration, and an attribute list with no specifiers and no declarator reached check_declares_something. `__has_attribute(fallthrough)` answers 1, so code that probes for it failed to compile. A lookahead (`at_attribute_declaration`) now recognises one or more attribute groups followed by `;` at statement level and at file scope, and parses them once with the existing attribute parser, discarding anything they would have left pending for a declarator. Diagnostics follow gcc 13: silent before a case, default or ordinary label or a brace; "not preceding a case label or default label" elsewhere; an error outside every switch; "empty declaration" for any other attribute; -Wattributes warnings for a parameter, an ignored companion attribute, and the file-scope form. The "plain or __name__ spelling" test is now one function, Attribute::is_named, used by the helpers that each had a copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…roject's bounds Every entry checked against c17 and gcc 13. Fixes the 920728-1, return-mismatch, _FORTIFY_SOURCE, float-folding, SIMD-macro, max_align_t, implicit-int and -fgnu89-inline entries; points each torture skip row at its list in scripts/c17_torture.sh, which the copies here had drifted from; records the gcc-internal-builtin, one void arm and #cpu skips. Removes entries that were not decisions (the #__VA_ARGS__ spacing conformance bug, a false `used` row, a missing non-NFC warning) and history wording. The SIMD predefines stay, with the dispute stated as two positions and the code following one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nment rule
`packed` was read only on the struct-or-union specifier, so on a member
it was parsed and dropped: `struct { char a; int b
__attribute__((packed)); }` was 8 bytes where gcc makes it 5. Layout
also folded struct-level `packed` and `#pragma pack` into one cap,
which is wrong twice over: gcc's struct `packed` is `packed` on every
member, so a member's own `aligned` still raises it, and a pragma caps
a written `aligned` too (and was not applied to unions at all).
TypeTable::member_alignment is now the one rule, used by struct and
union layout and for bit-fields: 1 if packed, else the type's
alignment; raised by any alignment written on the member; then capped
by the pragma. StructMember::explicit_align becomes `align:
MemberAlign { written, packed }`, filled through a pending_packed slot
beside the existing alignment one, and the four copies of the
"is this packed?" test are AttributeList::has_packed.
Two bugs on the same path are fixed with it: `aligned(8) int b, c;`
in a struct aligned only b, and a bit-field after an `_Alignas` member
was rejected as "_Alignas cannot be applied to a bit-field". A
bit-field with a written alignment is now placed on that boundary.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does The bit builtins went through parse_unary_builtin, which wrapped the argument as written: no prototype, no argument check, no conversion. `__builtin_ctz(8.0)` counted the zeros of the double's representation (0 on x86-64, 2 on aarch64; gcc answers 3), and a struct argument was accepted. `__builtin_ffs*` declared a stand-in `unsigned long` parameter and never checked it. parse/bit_builtin.rs holds one table of the bswap, ctz, clz, clrsb, popcount, parity and ffs families with gcc's parameter and return types; each call is checked by the same check_call an ordinary call gets and its argument converted by convert_operand. The ffs rows call the library ffs/ffsl/ffsll through their real prototypes. check_call now names the callee when the call spelled one, so an argument diagnostic reads as gcc's does: "incompatible type for argument 1 of '__builtin_ctz': ...". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gcc folds every bit builtin of an integer constant, so it may initialize a static object, label a case, bound an array, name an enumerator or appear in _Static_assert. c17's constant evaluator had no case for any of them, ffs was always a library call, and only popcount (and parity, built on it) was folded -- by the optimizer, after those contexts had already rejected it. ctz and clz could not be folded even there: their 64-bit forms, and the clz inside clrsb, recorded the operand's width as their int result's. constfold::eval_bit_op is now the one arithmetic for every bit builtin, called by the constant evaluator (on the argument after conversion to the parameter type, so ctz(-1) reads UINT_MAX) and by eval_unop, which sccp and instcombine use for all the bit opcodes, bswap included. ffs of a constant becomes its value. ctz, clz and popcount are built by one linearizer helper that records an int result and the operand's type and width; Opcode::is_bit_count joins reads_another_type and the validator. ctz(0) and clz(0) fold to the operand width, as gcc 13 folds them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
assignment_fault returned early when either pointee was void, before the qualifier check, so `void *q = (const int *)p;`, passing a `volatile int *` to a `const void *` parameter, and `int *q = (const void *)p;` were all silent. C17 6.5.16.1p1 requires the target's pointee to carry every qualifier of the source's in the void case as in the compatible one, and gcc warns on each. They now get the existing "discards a qualifier from the pointer target type" warning, in assignment, initialization and argument passing alike. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oid * The builtin parsed its alignment and misalignment arguments, threw both away, and returned the pointer as written: `k++` in either position never ran, the result kept the pointer's own type, and it was even an lvalue. gcc's prototype is `void *(const void *, size_t, ...)`. parse/assume_aligned.rs checks the call against that prototype with the shared check_call and converts the pointer with convert_operand; a non-literal alignment or misalignment is kept, in a comma expression ahead of the pointer, so it is evaluated. gcc's own extra rules -- at most three arguments, an integer misalignment -- are diagnosed in its words. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ger type C17 6.7.2.2p4 makes every enumerated type compatible with one integer type. The parser chose gcc's (unsigned int when no enumerator is negative, int when one is, long or unsigned long past 32 bits) but recorded it only as the enum's size and UNSIGNED modifier, and nothing turned that back into a type: an enum was compatible with no integer type, so `_Generic((enum E)0, unsigned: ...)` fell to default and __builtin_types_compatible_p(enum E, unsigned) was 0. Arithmetic had the same gap -- `e + 1` kept the enum's type and every enum ranked as int, so a 64-bit signed enum plus 1u came out unsigned rather than long -- and diagnostics printed `unsigned enum E`. TypeTable::enum_compatible_type reads the integer type back. The one compatibility rule swaps an enum for it against a non-enum (two different enums stay incompatible), and integer_promote and integer_rank go through it, so `e + 1` is gcc's type. A typedef may be redefined only to the same type (6.7p3), which is narrower than compatibility: types_same is the same rule with top-level qualifiers significant and an enum distinct from its integer type. It keeps `typedef enum E T; typedef unsigned T;` an error and closes `typedef int T; typedef const int T;`, which was accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eir real prototypes Twenty invalid builtin calls that gcc rejects compiled silently: a classification builtin of an int, __builtin_complex of integers, an overflow builtin writing through a double *, a variable object_size type or prefetch hint, an atomic on a double or a non-pointer, a non-constant always_lock_free size, va_start in a function with fixed arguments, and wrong-arity or wrong-type calls to library builtins when no header declared them. The last were declared through placeholder prototypes (every parameter unsigned long, libm types guessed from the name) whose calls were never checked. That machinery is deleted: every library builtin has a row in LIBRARY_BUILTINS with the library's real prototype -- libm, nan*, glibc's _chk functions, the allocators, the string helpers -- and is declared and checked from it like any call; -fpermissive implicit declarations take their return type from the same table. parse/builtin_args.rs holds the shared checks (through gcc's prototype, an integer-constant argument in a range, a floating argument) and parse/generic_builtin.rs the families that use them. Arity errors now read as gcc's, "too few arguments to function 'f'", for every call; "call expects N arguments" is gone. __atomic_test_and_set and __atomic_clear operated on the whole pointee rather than its first byte: on an unsigned holding 0x100, test-and-set returned 1 and left 0x1 where gcc returns 0 and leaves 0x101. They now act on the byte at the pointer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The harness ignored dg-error, so a test gcc's suite marks as invalid counted as a pass when c17 accepted it and as a regression when c17 rejected it correctly. dg_scan now reports the lines of every dg-error that applies to the run (target selectors evaluated by the sel_eval that dg-skip-if already used, with size32plus and int32plus added as effective targets), and such a test is compiled once with -S and never run: it passes only when c17 exits non-zero with its errors on exactly the marked lines. The message text is gcc's and is not compared; a dg-error form the suite does not use is skipped as unrecognised rather than read wrongly. compile/pr83547, pr48767, pr28865 and 20030305-1 leave both baselines: c17 accepts all four, each of which gcc rejects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e operator
make_binary rejected only void, vector, and complex-relational operands
and then let common_type answer for the rest, which it did even for a
struct: `i = s + 1` was reported only as an assignment of the wrong
type, and `(void)(s + 1)` not at all. ~, !, ++/--, the first operand of
?:, if/loop conditions and compound assignments were not checked;
`z % 2` and `z << 1` on a complex value surfaced only in the linearizer
as a line-0 "unsupported operation".
parse/operand_rule.rs holds the constraints as pure questions about
types: five operand classes (integer, integer-or-complex for gcc's `~`
conjugate, real, arithmetic, scalar), one table from each binary
operator to its class plus the pointer forms of + - relational and
equality, and one from each unary operator to its class. expr_check
reports them in gcc's words -- "invalid operands to binary + (have
'struct S' and 'int')", "wrong type argument to unary minus", "used
struct type value where scalar is required", "comparison between
pointer and integer" -- and leaves a failed operator's result untyped,
so each mistake is one error. A compound assignment is checked as its
operator, then as the conversion of the result.
compile/pr83547 returns to both torture baselines: its three
`({ ... })` conditions without a value are now rejected on the lines
gcc rejects them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… undefined enum The va_arg arm built its node from the type-name without checking it, so `__builtin_va_arg(ap, void)` and an incomplete struct were accepted though C17 7.16.1.1p2 requires a complete object type. They now get gcc's "second argument to 'va_arg' is of incomplete type '...'" (and "... is a function type" for a function type), and a type that promotes through `...` -- _Bool, char, short, float -- gets gcc's "'X' is promoted to 'Y' when passed through '...'" warning. The completeness test for a type-name lived inside the sizeof check; it is now Parser::type_name_is_incomplete, used by both. It also covers a forward-declared enum, so `sizeof(enum E)` with E undefined is rejected as gcc rejects it. compile/pr48767 returns to both torture baselines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…give it every elided value
walk_struct_init_fields, which both initializer paths use, did not know
where a struct sat in the object being initialized or whether that
object was static, so a flexible member's initializer was laid out
anywhere: inside an element of an array of such structs (each element a
different size, which no array can hold), and in an automatic object,
whose stores went past its end.
fam_init_violation is now the one rule, applied right after the walk in
both paths, with gcc 13's verdicts and words: in an automatic object any
initializer of the member is "non-static initialization of a flexible
array member"; in a static one a string is rejected only inside an
array element and a list of values everywhere but the object's own top
level, as "initialization of flexible array member in a nested
context", once per element. `{}` and the top-level GNU form stay
accepted.
With braces elided, a flexible member took only the first remaining
value: `static struct W w = {1, 2, 3}` stored 1 2 1 where gcc stores
1 2 3. It now takes every remaining value.
compile/pr28865 and compile/20030305-1 return to both torture
baselines.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ever it is written C17 6.7.6.2p1 forbids an array whose element type is incomplete or a function. The only check, check_element_type_complete in bind.rs, ran when a declaration was bound, so type-names (sizeof, _Alignof, casts, va_arg, compound literals), parameters, members and a file-scope extern never reached it, and it missed arrays of void, of functions and of unsized arrays (`int a[3][]`). `int a[2](void)` was even built as a function returning an array. check_array_element now runs where every declarator's array extents are applied, so declarations and type-names share one rule, reusing type_name_is_incomplete, with gcc's words: "array type has incomplete element type 'T'", "declaration of 'x' as array of voids|functions", "type name declared as array of ...". parse_array_extent reports an Extent -- constant, run-time or `[]` -- so `int a[3][]` is told from a VLA and `[*]`; a grouped declarator's inner array is checked once its real element type is known; the function suffix is applied before the extents. The bind-time check and the then-unused TypeTable::array_element_deep are deleted. Still accepted where gcc rejects it: an array of a typedef that is itself an unsized array (`typedef int A[]; extern A x[3];`), which the type table cannot yet tell from a block-scope VLA typedef. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The order was lost on the way to the back ends: the __atomic_* read-modify-write builtins evaluated it and dropped it (emit_atomic_rmw and its CAS loop were always seq-cst), the compare-exchange failure order was parsed and dropped, and only an integer literal counted as an order. aarch64 then ignored it anyway: every load was ldar, every store stlr, every exclusive loop ldaxr/stlxr. The order now reaches every atomic emitter, and each target maps it to instructions in one function, as gcc 13 does: on aarch64 ldr/ldar, str/stlr, and ldxr/stxr with the acquire half only when the order acquires and the release half only when it releases; on x86-64 plain mov for loads and non-seq-cst stores, xchg for a seq-cst store, a lock prefix for every read-modify-write. A non-constant order is seq-cst, any integer constant expression is an order, and an order the operation cannot take becomes seq-cst with gcc's -Winvalid-memory-model warning; a compare-exchange combines its success and failure orders by gcc's rules. The six near-copy aarch64 exchange/fetch emitters are one LL/SC emitter, and one emit_atomic_cas builder serves the IR side. Two fences were wrong: an aarch64 release fence was `dmb ishst`, which orders only stores against stores and so is no release fence (now `dmb ish`), and x86-64 acquire and release fences emitted lfence and sfence where nothing is needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
__atomic_signal_fence and __c11_atomic_signal_fence (what <stdatomic.h>'s atomic_signal_fence maps to) became the same Fence as a thread fence, so both back ends emitted mfence or dmb ish for an ordering that concerns only a handler on the same thread. gcc emits nothing at every order. The scope is carried as InsnExtra::fence_scope, beside the order. A field rather than an opcode keeps every pass predicate -- is_memory_barrier, has_side_effects, may_access_memory, the dce roots, the loadfwd and dse clobber arms -- reading the opcode alone, so a signal fence stays exactly the compiler barrier a thread fence is; Instruction::hardware_fence_order is the one question the back ends ask, and it answers None for a signal fence. The linearizer's two fence arms are one emit_fence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… access does get_mem_addr_for_atomic, used by exchange and the fetch-ops on x86-64, turned any stack location into `lea` of the slot without asking what the pseudo was. A spilled pointer's slot holds the pointer, so `__atomic_fetch_add(p, 2, ...)` added 2 to p itself and `__atomic_exchange_n(p, 9, ...)` overwrote p's low half: at -O1 and up the compare-exchange that followed faulted on p = 0x7fff00000009. A stack-passed pointer argument fell to the helper's default arm and became a null address. scalar_mem_operand (x86-64 memory.rs) is now the one rule: a register is the base, a Sym's slot is the object, any other pseudo's slot or incoming argument holds the pointer and is loaded, a global goes through TLS, the GOT or RIP-relative addressing. Ordinary loads and stores use it -- their assembly is identical across 22,308 compiles, Linux and Darwin, -O0, -O2 and -O2 -fPIC -- and every atomic emitter goes through atomic_mem_operand, which adds a scratch copy where the atomic sequence overwrites the base. The compare-exchange no longer spills its operands to the red zone, and get_mem_addr_for_atomic is deleted. aarch64's atomics take their addresses through the same compute_mem_addr rule as its other memory accesses; no live aarch64 path differed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parse_enum_specifier read the tag straight after `enum`, so
`enum __attribute__((packed)) A {...}` was a parse error, and attributes
after the closing brace were taken by the declaration rather than the
enum, so a trailing `packed` was lost (and a trailing `aligned(8)`
aligned the declared variable, where gcc ignores it). gcc gives a packed
enum the smallest of char, short, int and long that holds every value,
signed when one is negative, aligned to its size.
enum_underlying_type takes a `packed` flag and walks one candidate list,
starting at char when packed and at int otherwise. The attribute parsing
before a struct's tag is now parse_attributed_tag, which enums share,
and enums read their trailing attributes; between the tag and `{` stays
an error, as in gcc. aligned on an enum is ignored, as in gcc.
Two neighbouring defects: an enum wider than int gives its constants the
enum's own type in gcc 13 (`sizeof(L0)` is 8 for an enum with a value of
2^32), now set by enumerator_type when the enum completes; and
`enum E {}` is an error, as in gcc and clang, not a warning calling it a
GNU extension.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…width An array or a function used as a register operand of inline asm has decayed to a pointer (C17 6.3.2.1p3-4), but the operand took the object's size: `"r"(a)` for a 32-byte array was laid out at that width and the register came out 32 bits wide, `0(%eax)`, truncating the address. A value operand of array or function type now has the decayed pointer's type; a memory operand keeps the object's size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ms_abi was accepted and compiled as System V, so every call between a c17 ms_abi function and gcc's read its arguments from the wrong registers; calls were also classified by the caller's convention rather than the callee's, and on aarch64 sysv_abi selected the x86-64 classifier. The convention is now part of the function type (Type::conv): it is in the interning key and in compatibility, so ms_abi and sysv function types are distinct, and gcc's placement rule carries it to functions, pointers to them, typedefs, parameters, members and casts -- including `long (__attribute__((ms_abi)) *fp)(long)`, a spelling c17 could not parse for any attribute. CallingConv::of_callee is the one place a call decides its convention. abi/win64.rs is the classifier: four argument positions in rcx/rdx/r8/r9 or xmm0-3, the 32-byte shadow space, aggregates of 1, 2, 4 or 8 bytes by value and anything else (long double, __int128, __float128, wider complex) by a caller-made copy, a hidden return pointer in rcx returned in rax. The back end (arch/x86_64/win64.rs) lays out calls through their outgoing slots, spills a callee's register arguments to its shadow space, and saves rsi, rdi and xmm6-15 with their CFI. __builtin_ms_va_list and its start/end/copy builtins and va_arg are implemented as gcc names them. On aarch64 both attributes warn as ignored, as in gcc, and the native convention applies. address_of_pseudo took the address of an incoming stack slot whatever it held; a pointer parameter there -- a Win64 by-reference argument, or a System V 7th-position struct pointer forwarded to a by-value call at -O2 -- is now loaded instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
codegen_asm_array_operand_is_a_pointer spelled its aarch64 template as a plain Rust string, so `\n\t` became a real newline inside the C string literal and the program no longer parsed; the x86-64 template has no escape, which is why only the aarch64 and macOS runners failed. diagnostics_recognised_attributes_are_silent required __has_attribute(ms_abi) everywhere, but ms_abi is honoured only on x86-64 and, as in gcc, ignored with a warning and not claimed on aarch64. The check now asks that the answer match the target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
No description provided.