Conversation
optimize() rewrote the constant operand of a power-of-2 multiply, divide or modulo in place. rename_var() gives every use reached by one definition the same var_t, so that rewrite changed the value every other use of the constant saw, and it compounded: "int k = 8; return a * k + b * k;" returned 25 rather than 40. Reduced constants now get a variable of their own, defined by their own OP_load_constant, leaving the shared one alone.
Several buffers were written without checking the room left in them. The "#include <...>" loop appended every remaining token into a MAX_LINE_LEN buffer with no bound, for a path the FIXME beside it says is discarded. It now consumes the tokens without copying, and stops at a newline so an unterminated directive cannot run to end of file. Numeric and character literals are scanned up to MAX_TOKEN_LEN, so that is the width their destinations now have; MAX_ID_LEN bounds identifiers only, and copying a literal into one overran it. lex_peek_n() joins lex_ident_n() as the bounded form, and the switch case reads into a buffer of its own rather than the identifier buffer statement parsing shares. side_effect[] took three entries per postfix operator with no check, so a fourth in one statement wrote past it into the adjacent label and loop tables. dce_init_mark() wrote up to ten worklist entries before its caller tested for room. An implicitly sized initializer declared array_size == count while storing only the first 256 elements, which was a silent miscompile; it now reports instead.
A mistake in the source or on the command line reached abort(), so an ordinary typo ended in "Aborted" with no location, and several reached a NULL dereference first. usage_error() reports an invocation mistake and exits, leaving fatal() and its core dump for broken invariants. An undeclared name behind &, * or **, member selection on a scalar, a ternary missing its colon, and INT_MIN divided or taken modulo by -1 now name a file, line and column. Dead code after a return or goto no longer aborts the compiler, which is legal C that gcc accepts silently; the goto case asserting that abort now asserts the correct result. fopen() folds a negative errno into NULL, so the "if (!fp)" that elf.c and lexer.c already wrote works when shecc compiles itself.
__format_to_buf() had no case for %p, and an unmatched conversion writes nothing at all rather than failing, so a pointer silently vanished from its output. A pointer occupies VA_INT_STEP int-sized slots, so on an LP64 target the second one carries the high word; both are printed, in one spelling that does not depend on the value. printf() and fprintf() each staged into a 200 byte buffer with the clamping disabled, then wrote the length the conversion would have produced, so anything longer ran off the frame. They now share one helper that formats again into a buffer sized to the result rather than writing a truncated line.
The graph printer already existed but was excluded from self-hosted builds and only reachable through --dump-ir, which wrote CFG.dot and DOM.dot into the working directory as a side effect of dumping IR. --dot now writes the control flow graph as Graphviz DOT and stops, defaulting the file name from the input the way lacc does. It prints before unwind_phi() turns phi values into copies on the incoming edges, so the phi nodes are still in the graph, and prunes unreachable functions first so the embedded library does not dwarf the input. An output naming the input is refused, since the graph is written by truncating it and that destroyed the source. ssa_build() no longer calls unwind_phi(); the caller does, which is what gives --dot somewhere to stand between the two. The unused dominator dumper is gone rather than carried into the self-hosted build.
The snapshots pinned frontend IR shape for three programs across two architectures and two link modes. They cost two shell scripts, twelve checked-in JSON files, four Makefile targets that reconfigured the tree as a side effect, a CI job, a graphviz and jq prerequisite gate on every build, and a sed and jq pipeline whose job was to launder unstable output. That is a lot of machinery to protect three files. The same three programs now run in tests/driver.sh, under both the host built and the self-hosted compiler; the Makefile rule they replace only ever built them with stage 0. An empty expected output there asserts that the program prints nothing, rather than waiving the comparison. What is lost is real and worth stating: nothing now checks IR shape, so a frontend change that alters the graph while the programs still print the same thing goes unnoticed.
driver.sh compiled with stderr redirected to /dev/null, so a test that failed because shecc emitted a diagnostic reported only an exit code mismatch. It also stopped at the first failure, so one bad case hid the other six hundred, offered no way to run a single category, and left around 2400 temp files behind per invocation. It now shows the compiler's own output, reports every failure and still exits non-zero, honours TEST_FILTER and FAIL_FAST, and puts everything it creates in one directory that it removes unless something failed.
Every removal in peephole.c relinked its predecessor's next pointer and left the block's tail pointing at the node it had just removed. The x64 backend worked around that by walking each block to find the real last instruction, and said so in a comment. All twenty-nine removal sites now go through one helper that fixes the tail, which needs the block, so the passes that drop instructions take one. The walk is gone.
x64-codegen.c held both the instruction encoding and the IR walk, unlike arm-codegen.c and riscv-codegen.c, which include a separate encoder file beside them. A stale draft of that file existed but nothing compiled it, and it defined the ModRM mode constants with values contradicting the ones actually in use. src/x64.c now holds the encoding: REX and ModRM construction, the byte and dword primitives, addressing modes, patching, and the composed forms. It cannot have the shape of arm.c, because an x86-64 instruction has no fixed width and so there is no word to return; the helpers append to the code section and the caller composes them. Nothing in it reaches into the IR or the register allocator.
var_t carried a vreg_id nobody read or wrote, defs.h declared an elf32_dyn_t used for nothing and a MAX_BB_RDOM_SUCC that sized nothing, and two comments named an optimization the tree does not implement: opt-sccp.c is a constant cast pass, not sparse conditional constant propagation, and var_escapes() reports everything as escaping rather than deciding anything. MAX_IR_INSTR goes from 120000 to 262144. A self-compile emits about 101k, so the old ceiling left sixteen percent of headroom before an abort, and the slots are pointers in an arena that is never touched beyond what is used.
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.
The IR snapshot suite pinned frontend IR shape for three programs across two architectures and two link modes, and cost two shell scripts, twelve checked-in JSON files, four Makefile targets that reconfigured the tree as a side effect, a CI job, a graphviz and jq prerequisite gate on every build, and a sed and jq pipeline whose only job was to launder unstable output. This replaces that machinery with a
--dotflag that writes the SSA control flow graph as Graphviz DOT and stops, and moves the three programs intotests/driver.sh, where they now also run under the self-hosted compiler; the Makefile rule they replace only ever built them with stage 0.Making
--dotwork self-hosted turned up a wrong-code bug worth calling out separately.optimize()rewrote the constant operand of a power-of-2 multiply, divide or modulo in place, andrename_var()gives every use reached by one definition the samevar_t, so the rewrite changed the value every other use saw and compounded.int k = 8; return a * k + b * k;returned 25 rather than 40, on the host built and self-hosted compilers alike. It is the first commit here, isolated so it can be read and backported on its own.The rest is what the graph writer needed and what looking closely turned up:
%pin the embedded libc, whose formatter silently emitted nothing for an unmatched conversion; a bounded staging buffer shared byprintfandfprintf, which previously wrote past a 200 byte frame; bounds on several frontend buffers that could be overrun from ordinary input; diagnostics with a file, line and column where an undeclared name or a missing ternary colon used to fault or abort; and a split ofx64-codegen.cinto an encoder and an IR walk, matching howarm.crelates toarm-codegen.c.Verified on arm, riscv and x64, plus arm with dynamic linking:
make checkpasses 609/609 at stage 0 and stage 2 on each, the ABI suites pass, and stage 1 and stage 2 are byte-identical everywhere..ci/check-format.shand.ci/check-newline.share clean. Every commit builds, so the series bisects. Self-hosted--dotontests/fib.cnow produces the same 3470 line graph as the host build with no colliding node ids, where before the fix it collapsed 2234 nodes onto one.Deliberately not included: nothing now checks IR shape, so a frontend change that alters the graph while the programs still print the same thing goes unnoticed.
--dotoutput is deterministic apart from the pointer values in node names, so a normalized diff against a checked-in reference would restore that cheaply, without graphviz or jq, but that belongs in its own change.Summary by cubic
Replaces the IR snapshot suite with a
--dotflag that writes the SSA control flow graph as Graphviz DOT and exits, and moves the three snapshot programs intotests/driver.sh, where they now run under both the host and self-hosted compilers. This deletes twelve canned JSON files, two shell scripts, a CI job, and the graphviz/jq prerequisite from every build.a * k + b * kto compute 25 instead of 40.%pprints a whole pointer on LP64 instead of truncating it, and now renders the same way whether or not the pointer has a high word.--dotrun whose output name would overwrite its input, whether derived from a.dotinput or given explicitly with-o.src/x64.c, matching howarm.crelates toarm-codegen.c, and reports user errors with file, line, and column instead of aborting.driver.shnow surfaces the compiler's own diagnostics, reports every failure instead of stopping at the first, honorsTEST_FILTERandFAIL_FAST, and removes its temp directory unless something failed.MAX_IR_INSTRto 262144, since a self-compile emits about 101k.Note: nothing now asserts on IR shape, so a frontend change that alters the graph while output stays identical would go unnoticed.
Written for commit a8dc9e5. Summary will update on new commits.