Repository navigation
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
There was a problem hiding this comment.
Review summary
PR #2694 ("WIP: Diagnose modernc.org/libc ABI0 wrappers", #2691) adds one translation test plus a replace that redirects github.com/xgo-dev/plan9asm to a personal fork. The test is well-structured and consistent with the package's existing tests (mustTestPackage, TranslateSourceModuleForPkg, defer tr.Module.Dispose(), llvm.VerifyModule), and the ABI0 frame arithmetic ($56-52) is internally consistent. The two concerns are the committed fork redirect and a brittle/under-specified IR assertion — both inline below.
Findings: 1 merge-relevant (go.mod fork replace), 2 test-quality. No performance issues found.
Not placeable inline (outside the diff): THIRD_PARTY_NOTICES.md:70 still lists only github.com/xgo-dev/plan9asm, while the effective module graph (go.sum) now builds github.com/cpunion/plan9asm — the provenance notice no longer matches what ships.
Not buildable locally (LLVM headers unavailable), so this was judged by reading, not by running.
|
|
||
| replace github.com/xgo-dev/llgo/runtime => ./runtime | ||
|
|
||
| replace github.com/xgo-dev/plan9asm => github.com/cpunion/plan9asm v0.0.0-20260929013439-34a93b2081cf |
There was a problem hiding this comment.
P1 — a committed replace redirects a first-party compiler dependency to a personal fork. This shadows the github.com/xgo-dev/plan9asm v0.6.1 requirement for every build of the module (not only this test), moving control of compiled codegen from the xgo-dev org to an untagged personal-fork pseudo-version (v0.0.0-...). The shipped code differs from the org release — the module zip hashes differ (fork h1:6gLAPwaW... at go.sum:1 vs org v0.6.1 h1:WcwHdcNK... at go.sum:29) — so this is an unreviewed divergence on a compiler path, not a no-op. The replace also silently overrides any future go get -u of the real module until removed. For a WIP diagnostic, prefer a non-committed override (e.g. go.work) or upstream the fork and bump the require to a tagged release before merge; the fork redirect should not land in committed go.mod.
There was a problem hiding this comment.
Agreed: this replacement must not land on main, and I am leaving this P1 unresolved. I verified the latest official release is still v0.6.1 and reran the regression using a separate modfile without the fork override: it fails with unsupported struct type struct{Lo uint64; Hi uint64}. Removing the override now would break the regression; skipping it would hide the gap. The PR body now explicitly gates merge on an upstream dependency containing the fix, together with the API/ABI0 integration and execution tests. 71d85c0 also addresses the separate provenance finding in THIRD_PARTY_NOTICES.md; the pinned fork LICENSE is byte-identical to the bundled Plan9Asm license. This documentation does not resolve the dependency blocker.
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
5cbf9da to
f1c1571
Compare
f1c1571 to
ba60fff
Compare
ba60fff to
1470532
Compare
Refs #2691, #2708, #2707, xgo-dev/plan9asm#40 and xgo-dev/plan9asm#44.
Behavior
Compile selected Plan9 assembly with the same SDK, target, overlays and Go feature selection used by LLGo. Preprocessing, signature discovery, native routing and translation consume the same selected source. Missing or changed ownership is an error; an inferred declaration does not establish an ABI contract.
Current revision and dependency
Keep draft. Current contribution head:
7be828f55b9005f13f768b4a8a07aae4c57e6798, rebased onto upstream main610075be034817b0f386d109926fd9a93db41a48. Its own new CI and independent raw/native/profile/coverage acceptance are complete: 13 workflows, 140 SUCCESS +2 exact conditional publisher SKIPs, zero FAIL, attempt1, no reruns/cancellations. Codecov patch 97.06% >=95.82%. Previous F1 CI remains historical.Rebase retains the contribution and upstream ESP archive/system-package split. Full policy testing exposed an obsolete four-build assertion; the repair now checks all six exact build identities and their separate archive/package consumers. The actual failed check remains retained.
The pin changes only
go.modandgo.sum. Normal Go 1.27.3 authenticated the public module throughsum.golang.org, with no private/insecure/exclusion bypass. After the pin, exact module selection andgo mod verifypassed. Module checksum:h1:VxdavuA/5JTs9ipbB3PxoQ52QzEwJeZwQ7jEdCNfmP8=. All 5,273 module files match the 5,484-file Git archive, excluding exactly 211 nested CLI-module files. Proxy ZIP, extracted content and Apache notices agree; there is no local-directory replacement. An initial direct-network timeout is retained separately; retry used the existing system proxy, without changing authentication.Pinned translator 9285ff89a completed its own Go CI naturally on the first attempt: 325/325 SUCCESS, no reruns or cancellations. Official function coverage 88.1%; exact-head Codecov patch 91.80% >= 87.83%. Independent exact-head source/raw/profile/runtime acceptance is complete:286 source artifacts,240 partitions,23 groups; Windows/race22 packages and168 raw streams each;71 required runtime PASS nodes;289 ZIPs full SHA/size/CRC;22 profiles independently match official function coverage. Its completed x86 source inventory joins 4,986/4,986 current and 4,985/4,985 minimum identities; source joins do not mean complete IR/object/runtime coverage.
Before merging, incorporate and pin a tagged upstream release of
github.com/xgo-dev/plan9asm. The authenticated fork pseudo-version does not satisfy the unresolved official-dependency P1. Keep that thread unresolved and retain draft status.Fresh local validation
Compiler execution below binds AAED/latest main610/new9285 dependency, with frozen source, tools, selectors and raw outcomes. Changes since that qualified compiler are limited to CI dependency transport, the Windows GC fixture and its two-mode invocation, the release recipe/validation/documentation, and benchmark JSON transport/validation. Every normal compiler/library/header/module semantic input remains byte-exact; the Windows release recipe now omits DWARF. The changed C fixture is separately recompiled and executed in native Windows CI. This explicit bridge retains the actual AAED compiler qualification; it is not fresh Go/LLVM execution on this child.
The three host skips are precisely
TestCgoTypesLLVMLayoutfor wasm32 Emscripten, wasm32 WASI and wasm64 Emscripten; they are not passes. Host object/layout checks do not establish target execution.Linux ran in an owned Docker linux/amd64 container under Darwin/arm64 emulation, not native cloud hardware. All required executions passed in 259.235 seconds; its container exited naturally and was removed. The complete source tree, immutable15,657-file SDK, sixteen compatibility-runner files, new compiler and drivers remain hash-bound. Only compiler/module caches were reused; all compiler/probe outputs and execution receipts are new. A separate strict raw auditor checks exact RUN/terminal sets, numerical stdout, build info, source/tool identities and cleanup. This is not a claim of a new independent human review or all-green compatibility-repository CI.
CI dependency-installation repair
The preceding BA60 round exposed repeated Ubuntu archive TLS failures before Go tests. Preserve that failed round; do not call those setup errors instruction or ABI failures.
The shared mirror-list writer now separates architecture endpoint selection from transport selection. amd64/i386 use Ubuntu archive/security; other Ubuntu architectures use ports. Both receive the same HTTP priority1/HTTPS priority2 mechanism. APT Release signatures, package hashes, signing-key paths, strict update errors, retries and deadlines are unchanged; no insecure/unauthenticated or TLS-verification bypass is added. Source suites/components and third-party repositories remain untouched.
Actual TDD retains two x86 failures, then two ports failures, followed by three passing unit methods covering all four architecture cases. Final six complete policy suites pass107 methods. Real pinned Ubuntu24 containers deliberately fail their first HTTPS endpoint, then successfully fetch four signed Release documents each for amd64 and arm64 ports. All eight signatures are independently checked against the original Ubuntu keyring; both containers exit0 and are removed. The ARM64 metadata check uses an amd64 APT client and proves transport/profile behavior, not ARM64 machine execution. These bounded tests omit package indexes; an earlier full-index fixture hit its240-second budget and remains failed. A separate nominal check also passes in9.600 seconds: full selected Ubuntu indexes, four authenticated Release documents and two real zlib
.debdownloads. It does not install packages. This head's full Ubuntu dependency and cross-compiler jobs now pass, including the amd64 lane that previously rejected inconsistent signed mirror indexes; no trust checks were weakened.Windows GC fixture readiness repair
Native thread capture reproduced the original Win32 timeout: the collector's pre-stop callback held the GC lock while waiting for a heap helper that had not finished
GC_register_my_thread. A returned thread handle did not imply readiness. Wait for both registered user routines before starting collection; retain the original heap-lock interval, timeouts and exception-handler assertions. The production GC patch is unchanged.An explicit 100ms registration delay fails twice before the repair, with the same blocked-registration stacks. The final committed fixture passes205 native Windows executions: 200 heap-lock executions and five explicitly separate no-heap controls, including70 uninstrumented original-source executions and30 delayed registrations. Two separately classified broken-barrier controls fail with exits5/6 and matching registration-deadlock stacks; they are not passing fixture executions. No positive failures, skips, timeouts or process kills. The ordinary CI job now runs both normal and intentionally delayed registration. Temporary thread-dump and diagnostic workflows remain only in the fork branch. Prior250 passing executions did not reproduce the race and were not called a repair.
Windows release package budget
The complete BA60 round also exposed a real packaging failure: the386 MSVC ZIP was136,522,351 bytes, exceeding the unchanged128MiB budget by2,304,623 bytes. Increasing ZIP compression saved only396,133 bytes, so it was not used as the repair.
Release host compilers use Go's
-wlinker flag; the symbol-removing-sflag is not used. SDK/runtime payloads and licenses remain. A shared flag constructor feeds the production build and a real cgo regression. The native test first fails without debug omission.Full147 CI exposed a test assumption: MSVC external linking need not emit the COFF symbol table read by
go tool nm. Replace that check with actual execution of each host-architecture fixture, requiring the runtime function name, stack frame, module build info and embedded version/time linker settings. MinGW386, MSVCamd64 and MSVCarm64 pass six real build/link/execute cases, including normal and spaced library paths, no-DWARF inspection and the existing integrity rejections. No host skips. The earlier diagnostic's relative log-path failure remains failed; native Windows CRLF source hashes match the canonical files. The three actual production compiler jobs now pass too: their six real host build/link/execute fixtures retain runtime names and build metadata. The386 release producer runs its cgo fixture with the running amd64 Go tool; actual386 release execution is separately established by the completed386 installation jobs.The six actual new release ZIPs pass the unchanged128MiB budget, sidecar/SHA checks, full ZIP CRC, source-pinned ESP payload hashes and compiler checksums. Both ABI profiles use identical compiler bytes for each architecture; licenses and bundled ESP payloads are present.
The largest ZIP is 120.354MiB, with8,017,386 bytes below the original limit. All six archive installation jobs, actual native Go/C/C++ compilation/link/execution profiles and24 bundled ESP firmware builds pass. Firmware ELF checks are build evidence, not device execution.
prepare-wingetpasses with all six real archives. No already-published release was changed.Benchmark JSON transport
A complete local policy run reproduced a valid Go benchmark result split into separate name and measurement output events. The old runner rejected it as incomplete; a collector could also miss a duplicate whose first result was fragmented. One shared per-test line normalizer now feeds both consumers. Deterministic regressions cover partial names/metrics, interleaved packages, optional bench frames, multiple result lines, missing metrics and duplicate results. They fail before the repair and pass afterward; the real Go process/coverage fixture and all227 policy methods pass. Raw output is preserved, and incomplete/duplicate results still fail.
CI and review acceptance
The previous BA60 round naturally completed all13 workflows /142 jobs:119 SUCCESS,21 FAIL,2 exact conditional publisher SKIPs, with no reruns/cancellations. Its failures comprise18 APT setup errors, the GC fixture registration race, one dependent coverage gate and the package-budget error; all remain recorded.
The147 round also ended naturally:123 SUCCESS,4 FAIL,7 SKIP across134 materialized jobs. Three compiler jobs failed the same COFF-only assertion, one setup job rejected inconsistent signed Ubuntu mirror metadata. Twelve planned downstream matrix targets did not expand and are not counted as passes. Those failed/skipped results are retained.
This contribution's new round ended naturally and passed complete independent acceptance. The expected142 named jobs and actual GitHub Actions check IDs agree exactly, including every expanded target; only the two PR-inapplicable publisher jobs are skipped. No failed round, old artifact or old PASS is substituted.
Current Go CI, LLGo CI and Release CI bind contribution7be828f55 to actual merge82cf94e52; its tree exactly equals the contribution tree and main remains610075be0. Cloud profiles follow the committed Go1.27.0 pin; local qualification used Go1.27.3. These are recorded separately. Final independent acceptance SHA256:
165672d829a5bad27e42926d65e23b7b5b57c5fce85ce122f214f3387fb913f4.Keep draft: the only unresolved review thread is the official-dependency P1; the latest official plan9asm release/tag is stillv0.6.2. No formal approval is recorded. Platform/hardware exclusions and broader runtime gaps remain explicit.
Historical F1/d905 completed thirteen workflows and142 named jobs: 140 SUCCESS +2 exact PR-only publisher SKIPs, no failures/reruns/cancellations, patch97.06%>=95.84%. Its complete raw/native/profile acceptance remains retained but does not certify this contribution head.
Those broader probes used another compiler/translator and do not determine this head's behavior. Function boxes, callbacks, system stacks, native386 and AVX512/ER hardware acceptance require separate evidence. Bounded CRC/libc and ABI0 successes do not close every related issue or prove execution on every architecture.