aheejin accepted this revision. aheejin added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:180 +TARGET_BUILTIN(__builtin_wasm_load32_lane, "V4iIii*", "nU", "simd128") +TARGET_BUILTIN(__builtin_wasm_load64_lane, "V2LLiIiLLi*", "nU", "simd128") +TARGET_BUILTIN(__builtin_wasm_store8_lane, "vV16ScIiSc*", "n", "simd128") ---------------- `U` in the third argument [[ https://github.com/llvm/llvm-project/blob/72732acade77d5ee55a818e2da77a2c5b7033ccb/clang/include/clang/Basic/Builtins.def#L75 | means ]] pure. Can we say loads are pure? (The same for existing `__builtin_wasm_load32_zero` and `__builtin_wasm_load64_zero`) ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16782 + return Builder.CreateCall(Callee, {Vec, LaneIdx, Ptr}); + } case WebAssembly::BI__builtin_wasm_shuffle_v8x16: { ---------------- Would it be better to follow the arguments order in the original proposal? The order in [[ https://github.com/WebAssembly/simd/pull/350 | here ]] is mem, v, and lane. (I don't know why loads need `v` though) The same for intrinsic and instruction. ================ Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:246 + +// TODO: Also support the oher load patterns for load_lane once the insructions +// are merged to the proposal. ---------------- ================ Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:315 +defm : StoreLanePatNoOffset<v2i64, int_wasm_store64_lane, LaneIdx2>; + //===----------------------------------------------------------------------===// ---------------- ================ Comment at: llvm/test/CodeGen/WebAssembly/simd-load-lane-offset.ll:4 + +; Test SIMD v128.load{8,16,32,64}_lane instructions. TODO: Use the offset field. + ---------------- Maybe add a comment on which patterns are currently supported and unsupported among this file? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89366/new/ https://reviews.llvm.org/D89366 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
