Repository navigation
simd: expand SIMD128 operations and runtime integration - #2722
Merged
xushiwei merged 20 commits intoOct 6, 2026
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
SIMD128 implementation review
Reviewed the SIMD128 lowering, Emscripten SjLj bridging, CPU-detection patches, and GOAMD64 wiring across the 783ec4f...a09e36e range. The implementation is strong overall: poison-avoidance (shift clamping, permute index masking, saturating fptosi.sat/fptoui.sat, compile-time lane-index checks), nil-deref guards on load/store, and the "vector-bits-only" GC reasoning for the Emscripten stack slots all check out. The package-dedup fix (keying checked by pkg.ID instead of PkgPath) and the single non-cumulative GOAMD64_vN macro injection both match the intended semantics.
A few mostly-minor items are noted inline. Nothing blocking.
Notes considered but not flagged:
- The empty-slice/NUL-termination assumptions in
sysctl_darwin_llgo.goare carried over verbatim from the deletedos_darwin.go, fed only by fixed stdlib string literals — preserved behavior, not a regression. requireSIMDFeatures/finishSIMDFeaturescost is pre-existing (unchanged by this PR), so not in scope here.
|
|
||
| The current WAMR 2.4.5 classic-interpreter profile rejects `v128` function | ||
| types with `unknown value type`, even though its build reports SIMD enabled. | ||
| The WASI suite is therefore blocked at module loading; the same failure is |
Contributor
There was a problem hiding this comment.
[P2] README says WASI suite is blocked, but CI still runs it
This states the WASI suite is "blocked at module loading" on the WAMR classic-interpreter profile. However the Running section just above (llgo test -O2 -target wasi -emulator ... ./test/simd/...) and CI (.github/workflows/llgo.yml:898-901) actively run exactly that LLGo WASI SIMD invocation. If the v128 module-load failure is real on the shipped WAMR profile, that command/CI step would fail at load. Please reconcile: either the suite does run (clarify under which engine) or the command/CI step should be marked skipped/expected-fail.
| n := x.ll.VectorSize() | ||
| mask := llvm.ConstInt(indices.ll.ElementType(), uint64(n-1), false) | ||
| result := llvm.Undef(x.ll) | ||
| for i := 0; i < n; i++ { |
Contributor
There was a problem hiding this comment.
[P2] Dynamic Permute emits a scalarized per-lane gather
For SIMDPermute/SIMDPermuteOrZero the fallback emits, per lane, extractelement(index) + and + extractelement(value) + optional icmp/select + insertelement — a serial ~4-6 instruction chain through result (≈80-100 IR instructions for 16 lanes). LookupOrZero already uses the hardware wasm.swizzle/neon.tbl1 intrinsic, but the dynamic-index Permute path has none, so unless the backend recollapses this it is a runtime SIMD-quality regression. If indices are commonly compile-time constants, a single shufflevector would be far better. Worth confirming the optimizer folds this back to a vector shuffle.
| SIMDTrunc: "llvm.trunc", SIMDRound: "llvm.roundeven", | ||
| } | ||
|
|
||
| func (b Builder) simdFeatures(op SIMDOp) { |
Contributor
There was a problem hiding this comment.
[P3] simdFeatures ignores its op parameter
func (b Builder) simdFeatures(op SIMDOp) never uses op; it only forwards to requireSIMDFeatures(). This implies per-op feature logic that no longer exists. Consider dropping the parameter (or the wrapper) and calling requireSIMDFeatures() directly.
| info := simdLanes(x.RawType()).Elem().Underlying().(*types.Basic).Info() | ||
| var cond llvm.Value | ||
| if info&types.IsFloat != 0 { | ||
| pred := map[SIMDOp]llvm.FloatPredicate{ |
Contributor
There was a problem hiding this comment.
[P3] Per-call predicate-map allocation in simdCompare
simdCompare allocates and populates a fresh map[SIMDOp]llvm.FloatPredicate (or Int) literal on every comparison lowered, just for one lookup; simdIntegerIntrinsic (simd_integer.go:60) does the same with a map[SIMDOp]string. A package-level var (as already done for simdFloatUnary) or a switch avoids the per-call allocation. Minor.
| v = b.impl.CreateBitCast(x.impl, typ, "") | ||
| } | ||
| if n != result.ll.VectorSize() { | ||
| // Narrowing conversions leave the unused upper lanes zero on these targets. |
Contributor
There was a problem hiding this comment.
[P3] "Narrowing conversions" comment conflates element vs lane count
This branch runs when the LLVM lane count changes (e.g. Float64x2 -> Float32x4: n=2, result size 4). The element type narrows (f64->f32) but the vector lane count increases from 2 to 4, and the shuffle zero-fills the added upper lanes. Calling this a "narrowing conversion" that leaves "unused upper lanes zero" reads as if lanes are dropped. Behavior is correct; the comment wording is misleading.
|
|
||
| func (b Builder) simdPermute(op SIMDOp, x, indices Expr) Expr { | ||
| if op == SIMDLookupOrZero { | ||
| name := "llvm.aarch64.neon.tbl1" |
Contributor
There was a problem hiding this comment.
[P3] LookupOrZero has no amd64 guard, would emit NEON intrinsic
SIMDLookupOrZero selects llvm.aarch64.neon.tbl1 for everything except wasm. This is correct today only because archsimd exposes LookupOrZero solely on arm64/wasm, but simdOperations registers it with no architecture guard. If the method ever appears on amd64, this would silently emit an AArch64 intrinsic into an amd64 module (invalid IR) rather than failing. Consider an explicit arch check with a panic/unimplemented fallback for the unexpected case.
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 |
zhouguangyuan0718
force-pushed
the
codex/simd-next-20261002
branch
from
October 4, 2026 12:03
a09e36e to
f40606a
Compare
Collaborator
Author
|
@fennoai 请重新检视这个 PR 的最新提交 这个 PR 创建较早，请特别检查哪些兼容或补救逻辑已经被近期主干改动覆盖，是否可以删除或收窄。重点包括 Emscripten SIMD 调用桥接与新的异常处理/线程模式、WASI LTO、CPU 初始化，以及源码补丁与测试集成。也请复查 SIMD lowering 的正确性及之前的检视意见，给出具体文件、触发条件和验证依据。 |
Contributor
|
An internal error occurred. Please try again later. |
Collaborator
Author
Contributor
|
An internal error occurred. Please try again later. |
zhouguangyuan0718
force-pushed
the
codex/simd-next-20261002
branch
from
October 5, 2026 01:03
a2b7c8b to
c6f35c7
Compare
Collaborator
Author
|
@fennoai review Please review the current PR head |
Contributor
Review: PR #2722 @
|
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
2 participants
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.
SIMD128 programs can now load and store arrays/slices, perform arithmetic and conversions, compare/select lanes, and use integer shifts, saturation, lookups, and permutations across the applicable amd64, arm64, and Wasm APIs.
The lowering keeps LLVM vector values across calls and computations while preserving Go aggregate storage and layout. It handles bounds/nil checks, shift limits, NaNs, signed zero, conversion overflow, and architecture-specific floating min/max behavior. amd64 permutations stay within the compilation baseline: constant indices fold to vector shuffles at O2, while dynamic indices retain a per-lane fallback.
Runtime and build integration:
archsimdand user initialization, including the officialGODEBUG=cpu.*policy and platform hooks.TypeError: type incompatibility when transforming from/to JS.EMCC_CFLAGS; Memory64's explicit JS SjLj codegen retains the bridge.LookupOrZerolowering to arm64/wasm; unsupported targets use the existing intrinsic fallback.plan9asm v0.6.2(amd64: preserve XMM/YMM/ZMM register aliasing and upper-bit semantics plan9asm#45) for shared XMM/YMM/ZMM register storage. Add scalar-reference CRC folding coverage and Windows CPU override expectations matching its environment-initialization order.Rebased onto
ec9c2488b, including the current Wasmer runner, WASI LTO, and Emscripten native EH capability support. The SIMD lowering and CPU/build integration remain necessary on that baseline. The README now includes WASI Thin/Full LTO commands and GoJS boundary coverage.Validation of the baseline refresh and SjLj fix (Go 1.27.0, LLVM 22.1.8, Emscripten 6.0.8, Node 24.19.0, Wasmer 7.5.0):
internal/clangandinternal/crosscompiletests passed.cl,ssa, andinternal/buildpassed. Bridge regressions check GoJS and Emscripten, unchanged IR under native SjLj, late inlining, flag precedence, C++ link-driver arguments, and Memory64's retained JS codegen.Earlier validation retained from this PR:
cl,ssa,internal/packages, andinternal/plan9asmtests; native arm64 SIMD at O0/O2 and Full LTO; amd64 SIMD via Rosetta atGOAMD64=v1with CPU overrides; WASI SIMD at O0/O2; full Emscripten SIMD at O2; official Go native and WASI shared behavior tests.Limits: dynamic amd64 permutation performance remains future work; no hardware-specific dispatch is added. The full O0 Emscripten test executable exceeds Node's local-variable limit, so O0 uses the smaller boundary executable. Official Go 1.27.0 WASI intrinsics do not implement LLGo's nil-array panic behavior; nil assertions remain enabled for every LLGo target and official native Go. Full standard-library
internal/cputests still encounter the separately reproduced duplicate-package-symbol issue. SIMD reflection, portablesimdspecialization, general FMV, and 256/512-bit vectors remain outside this stage.Follows #2581; implements additional operations from #2568.