wasm: honor WASI LTO and preserve threaded exception lowering - #2726
Conversation
There was a problem hiding this comment.
Review summary
The change is well-targeted. The LTO-mode gating in crosscompile.go is correct, and TestUseWASILTOEnablesSjLjAtLink now cleanly asserts the new link/clang flags appear exactly when lto.Mode.Enabled() is true across Off/Thin/Full. The new Python harness builds both thin and full modes and validates the LTO backend object and merged bitcode, giving strong coverage of the fix. Security and performance passes found nothing noteworthy (build-time flags only, no runtime hot path, list-form subprocess calls with check=True and timeouts).
One comment-accuracy finding is inlined below. A minor CI nit (the new Test dev WASI Thin and Full LTO step omits the set -euo pipefail guard used by every other multiline bash step in this workflow, so a go build failure surfaces as a confusing "file not found" from the next line rather than the real build error) is optional to address.
| // Without an explicit exception model, codegen drops SjLj catch pads. | ||
| export.LDFLAGS = append(export.LDFLAGS, "-Wl,--mllvm=-exception-model=wasm") | ||
| // ThinLTO compiles Go modules independently of the C modules that | ||
| // carry these features. Preserve shared memory, TLS and Wasm EH. |
There was a problem hiding this comment.
[P3] Comment scopes flags to ThinLTO and misnames -mattr features
The comment says "ThinLTO compiles Go modules independently ... Preserve shared memory, TLS and Wasm EH", but the enclosing guard is ltoMode.Enabled(), which is true for both lto.Thin and lto.Full (internal/lto/lto.go), and the harness added in this PR exercises both modes — so these flags matter under full LTO too, not just thin. The feature list is also inaccurate: the actual -mattr value is +atomics,+bulk-memory,+exception-handling; there is no +tls attribute (TLS on wasm is emergent from atomics+bulk-memory). Consider rewording to cover both LTO modes and to list the real attributes (atomics / bulk-memory / exception-handling) so a future reader does not assume the flags are thin-only or inert under -lto=full.
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 |
1906f8e to
c98d59e
Compare
c98d59e to
68f6f92
Compare
cpunion
left a comment
There was a problem hiding this comment.
已独立审查当前 head 68f6f92715cd,并对照 #2721。建议补齐下面的 LTO 优化等级遗漏后,先合并本 PR,再让 #2721 rebase、去除重复的参数修复。
本 PR 的参数修复及验收测试值得采用,但不能完整替代 #2721:后者还修复 C-only deadcode 链接时错误纳入未构建 runtime 包、缺失 metadata 的诊断,并有 Full LTO 多线程 GC 验收。函数属性处理也不能仅凭 -mattr 就全部删除;下面区分了已经确认的 LLVM 后端边界与未复现的普通 LLGo 回归。
独立验证环境:macOS arm64、Go 1.27.0、LLVM/LLD 22.1.8、Wasmer 7.3.0。
TestUseWASILTOEnablesSjLjAtLink、TestLTOLinkerOptFlag和完整internal/crosscompileshort 测试通过。- 本 PR 新增的
dev/test_wasm_wasi_lto.py完整通过,包括 Thin/Full 实际 LTO 产物、dev Full-LTO 接口 metadata、GC/nogc 的 Goexit/defer/C longjmp/recover、init/main Goexit、uncaught panic,以及两种 LTO 下的 SIMD 测试。 - 额外构建并运行
wasm-wasi-threaded-gc的 Full LTO 用例,通过wasi threaded gc ok,覆盖 worker 根、STW、C 阻塞及 arena 扩容。 - 实际 LLGo SIMD + atomic 样例,以及 deadcodedrop、nogc/PCLN-none 变体通过。
本地运行使用 Wasmer 7.3.0;CI 中仓库固定 Wasmer 7.5.0 的相关 WASI 任务也已通过,二者不混同。
建议收敛为:后端提供默认特性,已有 target-features 的函数合并必要特性并保留 SIMD;这样可以缩小 #2721 的属性处理范围,而不是逐函数重复写全部默认属性。
| if ltoMode.Enabled() { | ||
| // Clang does not forward -fwasm-exceptions to the LTO backend. | ||
| // Without an explicit exception model, codegen drops SjLj catch pads. | ||
| export.LDFLAGS = append(export.LDFLAGS, "-Wl,--mllvm=-exception-model=wasm") |
There was a problem hiding this comment.
[P2] 请把选择的优化等级传给 wasm-ld 的 LTO 优化器
这里把 -O0 / -O3 复制进 clang 的最终链接参数,但没有生成 --lto-O0 / --lto-O3。我用当前 dev 编译器分别构建了 -O0 -lto=thin 和 -O3 -lto=full 的 println:最终 clang 链接参数有对应 -O,实际 wasm-ld 参数没有 --lto-O*。LLVM 22 wasm-ld 的 LTO 默认等级仍是 2,因此选择的优化等级在链接阶段没有生效。
默认值见 LLVM Driver.cpp。#2721 已有这部分修复,可直接复用本文件现有的映射:
if optFlag := ltoLinkerOptFlag(level); optFlag != "" {
export.LDFLAGS = append(export.LDFLAGS, "-Wl,"+optFlag)
}建议在 Thin/Full 两种模式下补上 O0/O3 及 Os/Oz 的参数覆盖,并断言关闭 LTO 时不加入该参数。当前 WASI 新测试只使用 O2,无法发现这个遗漏。
| export.LDFLAGS = append(export.LDFLAGS, "-Wl,--mllvm=-exception-model=wasm") | ||
| // ThinLTO compiles Go modules independently of the C modules that | ||
| // carry these features. Preserve shared memory, TLS and Wasm EH. | ||
| export.LDFLAGS = append(export.LDFLAGS, "-Xlinker", "--mllvm=-mattr=+atomics,+bulk-memory,+exception-handling") |
There was a problem hiding this comment.
选型边界:-mattr 不是已有函数 target-features 的合并或强制覆盖
这不是已经复现的普通 LLGo SIMD 回归;当前新增矩阵及我补的真实 Go SIMD/atomic 样例都通过。不过,不能据此认为这一行与 #2721 的属性补齐完全等价。
LLVM 22 的 getSubtargetImpl(Function) 会用已有函数属性代替 TargetFS;随后 coalesceFeatures 对模块内定义函数的特性取并集。因此,如果 LTO 后端模块中的定义函数都已有属性,仅传 -mattr 不保证 atomics/bulk-memory/EH 被保留。
我在 LLVM/LLD 22.1.8 上验证的最小输入如下:
target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
target triple = "wasm32-unknown-wasip1"
@state = thread_local global i32 0, align 4
define i32 @inc() #0 {
%p = call ptr @llvm.threadlocal.address.p0(ptr @state)
%old = atomicrmw add ptr %p, i32 1 seq_cst
ret i32 %old
}
declare ptr @llvm.threadlocal.address.p0(ptr)
attributes #0 = { "target-features"="+simd128" }用 LLVM 22 工具运行:
clang -c probe.ll -target wasm32-wasip1-threads -O2 -flto=thin \
-matomics -mbulk-memory -fwasm-exceptions -o probe.o
wasm-ld probe.o --no-entry --export=inc --shared-memory \
--max-memory=16777216 \
--mllvm=-mattr=+atomics,+bulk-memory,+exception-handling \
--mllvm=-exception-model=wasm --mllvm=-wasm-enable-sjlj \
--mllvm=-wasm-use-legacy-eh=false -o probe.wasm会报 --shared-memory is disallowed ... because it was not compiled with 'atomics' or 'bulk-memory' features;换成 Full LTO 也复现。去掉 shared-memory 限制再检查后端产物,可以看到 TLS 被去掉,atomicrmw 变成普通 load/add/store。把该函数属性改成 +atomics,+bulk-memory,+exception-handling,+simd128 后,shared-memory 链接通过,并保留 TLS/原子指令。
实际 LLGo 样例中的 init 等无该属性的定义让模块能够取得后端默认特性,所以那些样例通过,不代表这条 LLVM 边界不存在。我的建议是采用本 PR 的后端默认参数,同时把 #2721 收窄为只补齐已有属性的函数,并增加上述已有属性 + TLS/atomic 的实际后端回归测试。
cpunion
left a comment
There was a problem hiding this comment.
复核新提交 db70b67550c0:未发现新增的合入阻塞,建议按原顺序先合并本 PR,再精简 #2721。
上轮意见的处理情况:
- LTO 优化等级遗漏已修复。 新实现直接复用
ltoLinkerOptFlag,只在 LTO 开启时加入链接参数;关闭 LTO、Thin/Full × O0/O1/O2/O3/Os/Oz 的 18 组配置测试均通过,且断言不会加入重复或错误的 optimizer flag。 - 函数属性的替代边界已说明清楚。 新注释和 PR 描述准确区分后端默认特性与已有函数
target-features的合并。本 PR 没有宣称替代 #2721 的属性处理;这一项继续由 #2721 补齐已有属性并添加后端回归测试,不作为当前参数修复的合入阻塞。 - 原注释的 Thin/Full 范围及特性名称已修正,新增 CI 步骤也补了显式 shell guard。新增 diff 集中,没有发现不必要的功能改动。
本轮独立验证(Go 1.27.0、LLVM/LLD 22.1.8、Wasmer 7.3.0):
- 构建新 head 的 dev 编译器,相关 flags 测试及完整
internal/crosscompileshort 测试通过。 - 对
globaldce_interface_matrix分别运行 Thin/Full × O0/O3 四组实际构建及执行;在 clang-v输出的实际 wasm-ld 命令中确认--lto-O0/O3,验证 LTO 后端产物非空,全部接口输出正确。两组 Full LTO 还检查了 merged preopt bitcode 中的llvm.type.checked.load,确认保留 dev GlobalDCE 路径。 - 最新提交的 CI 已完成:97 项成功,1 项 release 跳过,无失败;包括使用仓库固定 Wasmer 7.5.0 的实际 LTO/SIMD/EH 验收。当前 PR 无合并冲突。
#2721 仍保留 C-only deadcode 链接与 metadata 诊断、已有函数特性合并和 Full LTO 多线程 GC 验收;本 PR 合并后删除其中重复的参数修复即可。
WASI accepts
-lto=thin/fullwithout passing-fltoto Clang. Ordinary builds silently produce native Wasm objects, while dev Full LTO emitsllvm.type.checked.loadfor Go GlobalDCE and crashes during premature object code generation (Do not know how to promote this operator!).Pass the selected LTO mode through WASI compilation and linking. Forward the requested optimization level to wasm-ld with
--lto-O0through--lto-O3;Os/Ozuse--lto-O2with size attributes retained in bitcode. Without this forwarding, wasm-ld uses its default O2 optimizer even when the user selects O0 or O3. No LTO optimizer option is added when LTO is disabled.Also pass the Wasm exception model and atomics/bulk-memory/exception-handling defaults to the link-time backend: otherwise Full LTO drops SjLj catch pads, and independent ThinLTO modules lose shared-memory/TLS support or fail to select EH instructions. Keep standard Wasm EH and Go GlobalDCE method-retention metadata enabled.
Add off/thin/full flag coverage across O0/O1/O2/O3/Os/Oz and a CI regression using a dev compiler and the shared Wasmer runner. The regression verifies actual LTO backend objects, merged Full LTO bitcode, and dev type-checked-load metadata. Under both Thin and Full LTO, it runs interface dispatch, SIMD tests, and GC/nogc Goexit, defer, panic/recover and C longjmp across pthreads, including expected init/main Goexit and uncaught panic failures. Wasmer chooses the backend automatically, its module cache stays enabled, and engine tracing is suppressed for test-output checks. The multiline CI step explicitly enables
set -euo pipefail.Scope and related work:
3a8054a87, including the merged Wasmer migration (wasm: switch threaded WASI runner to Wasmer #2725) and browser coverage OIDC fix (fix(ci): allow OIDC for browser coverage uploads #2728).-mattrsupplies backend defaults; it does not merge required features into an existing functiontarget-featuresattribute. An LLVM 22 TLS/atomic probe with only+simd128on its definition fails shared-memory linking in both Thin and Full LTO despite these defaults; merging atomics/bulk-memory/exception-handling into that attribute fixes the probe. This is a confirmed backend boundary, not a reproduced failure of the ordinary LLGo matrix below.Validation on macOS arm64 with Go 1.27.0, LLVM/LLD 22.1.8 and Wasmer 7.5.0:
internal/crosscompileshort suite.--lto-O*in the wasm-ld command; each build produced a nonempty LTO backend object and ran under Wasmer. The previous compiler's O0 ThinLTO invocation lacked--lto-O0.dev/test_wasm_wasi_lto.pypassed the complete Thin/Full LTO matrix, including SIMD, verified LTO artifacts, dev metadata, and all GC/nogc exception cases.git diff --checkpassed.The dev Full LTO promotion crash was also reproduced on unchanged main
783ec4fd3. The earlier ordinary Thin/Full LTO results reported in #2725 demonstrated flag acceptance only; the checks above exercise actual LTO. CI for the updated head is pending.