Repository navigation
wasm: enable browser LTO and qualify JS SIMD profiles - #2744
Conversation
There was a problem hiding this comment.
Review summary
This PR extends the SIMD CI matrix to JavaScript/WASM targets (GoJS, Emscripten, Emscripten Memory64) across off/thin/full LTO, adds an emscriptenArchiver() helper so bitcode is archived with the SDK's emar, and forwards -flto=<mode> to both compile and link for the Emscripten branch. The change is well-factored and the docs/comments are accurate.
Verification against the code:
dev/test_wasm_simd.shmatches the README description (3 profiles x O0 boundary + O2 off/thin/full,-pclntab=none), and setsGOEXPERIMENT=simdinternally.- The Emscripten LTO branch (
crosscompile.go) correctly appendsltoMode.ClangFlag()to bothCCFLAGSandLDFLAGS. emscriptenArchiver()is only consulted for the GoJS/Emscripten providers and keeps theLLGO_ARoverride first.
No security concerns found. A few minor, non-blocking notes are inline.
| return "" | ||
| } | ||
| if cc, err := exec.LookPath(c.crossCompile.CC); err == nil { | ||
| if ar, err := exec.LookPath(filepath.Join(filepath.Dir(cc), "emar")); err == nil { |
There was a problem hiding this comment.
Minor: this sibling-tool probe diverges from the existing siblingTool() helper used just below for llvm-ar. siblingTool handles the Windows .exe suffix explicitly via os.Stat, whereas exec.LookPath of an absolute path relies on %PATHEXT% semantics. The Emscripten SDK ships emar as emar.bat on Windows, so the two lookups can behave differently and the sibling-emar probe may fall through to the bare emar on PATH (which could be a different SDK than the resolved emcc). Consider reusing siblingTool(cc, "emar") after resolving cc, so Windows handling and the lookup strategy stay aligned with the neighboring llvm-ar logic.
| } | ||
|
|
||
| // Emscripten C++ bitcode can be newer than LLGo's linked LLVM. Its emar | ||
| // wrapper selects the SDK's matching llvm-ar for both objects and MRI merges. |
There was a problem hiding this comment.
Minor (doc clarity): the comment mentions only "Emscripten C++ bitcode", but emscriptenArchiver() also returns emar for WasmProviderGoJS. That is intentional (the GOOS=js path configures CC=emcc and defaults to WasmProviderGoJS), but a one-line note that GoJS also builds through emcc/emar would prevent a future "is the GoJS case a mistake?" question.
| }...) | ||
| if ltoMode.Enabled() { | ||
| export.CCFLAGS = append(export.CCFLAGS, ltoMode.ClangFlag()) | ||
| export.LDFLAGS = append(export.LDFLAGS, ltoMode.ClangFlag()) |
There was a problem hiding this comment.
Minor (consistency): this forwards only the base ClangFlag() (e.g. -flto=thin). The sibling WASI branch additionally emits the LTO linker opt level (-Wl,--lto-O2 via ltoLinkerOptFlag) and exception/feature defaults. If emcc is expected to inject the LTO backend opt-level and feature wiring itself, a brief comment stating that would clarify why this branch intentionally differs from the WASI branch's treatment.
| if ar := os.Getenv("LLGO_AR"); ar != "" { | ||
| return ar | ||
| } | ||
| if ar := c.emscriptenArchiver(); ar != "" { |
There was a problem hiding this comment.
Minor (perf, pre-existing pattern amplified): archiver() / archiveMerger() are called once per package in createArchiveFile, and on wasm/Emscripten builds emscriptenArchiver() now adds up to ~3 exec.LookPath PATH scans per package (CC, sibling emar, bare emar) on top of the existing llvm-ar scan. The result is deterministic for the lifetime of the context. Consider resolving the archiver once and caching it (guarded with sync.Once, mirroring the existing plan9asmOnce) to avoid hundreds of redundant PATH scans on large builds. Not blocking.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
4ba81bf to
6cb2cbe
Compare
Browser builds accepted
-lto=thin/fullwithout forwarding-fltoto compilation or final linking. Forward both the LTO mode and the requested LTO optimizer level for GoJS, Emscripten, and Emscripten Memory64. Emscripten 6.0.8 consumes-Ofor compilation and post-link optimization but leaves wasm-ld's LTO level at its default unless--lto-Ois supplied explicitly. O0–O3 now select the corresponding LTO level; Os/Oz use LTO O2 while retaining size attributes and post-link size optimization.Prefer the SDK's
emarfor package archives and MRI merges, preserving theLLGO_ARoverride. Resolve Windows SDK.battools withLookPath; tests cover a competing SDK on PATH as well as fallback and override behavior.LLVM 22's O3 argument promotion can also replace an internal SIMD bridge's pointer argument with
v128, despitenoinline/optnone. That recreates an unsupported JavaScript SjLj signature on Memory64. Make the bridge's vector argument loads volatile to preserve its memory ABI. A regression test reproduces the failure under argument promotion and both O3 pre-link LTO pipelines. The default JavaScript SjLj/Asyncify configuration remains in use.Add
dev/test_wasm_simd.shto the wasm test-command CI job. Each of the three JS profiles runs an O0 boundary executable, the complete O2 SIMD suite with LTO off/Thin/Full, and O3 Thin/Full LTO boundary executables. The small O0 executable avoids the full test binary's engine local-variable limit.Validation with Go 1.27.0, LLVM 22.1.8, Emscripten 6.0.8, and Node 24.19.0:
--lto-Olevel in every case.TestLaneBoundsin each, requalifying the three targets from wasm: experimental SIMD vector calls fail at Emscripten JS exception boundaries #2729 after simd: expand SIMD128 operations and runtime integration #2722.v128JavaScript imports to zero and passes in real Chrome. Full LTO browser boundaries for all three profiles and Memory64 Go/C++ Thin/Full LTO boundaries also passed during initial qualification.-flto. ShellCheck, shell syntax, and diff checks pass.The expanded initial
internal/buildrun reached the default 10-minute package timeout after 538 completed top-level tests without earlier top-level failures. The remaining 143 tests passed separately in 55.5 seconds. Affected tests were rerun after these changes; The previous head also passed the fullinternal/buildCI run and both Windows Go jobs, including the new.batfixture.Rebased onto main
1866f43f5. The affected compile/link, archive, and SIMD bridge tests pass after rebase. Main already contains the GOROOT deadline-test scheduling fix; its focused regression tests pass, and this PR no longer changes that test.Refs #2729.