[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

Thomas Lively via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 30 17:29:48 PDT 2020


tlively marked an inline comment as done.
tlively added inline comments.


================
Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned long long __u64x2
+    __attribute__((__vector_size__(16), __aligned__(16)));
----------------
sunfish wrote:
> tlively wrote:
> > sunfish wrote:
> > > Since this file is already including <stdint.h>, instead of `char`, `unsigned char`, `short`, `unsigned short`, etc., can these use `int8_t`, `uint8_t`, `int16_t`, `uint16_t`, and so on, for clarity? Also, this would avoid any possible behavior change under `-funsigned-char`.
> > Unfortunately not :( This change gives errors like
> > 
> > ```
> > error: cannot initialize a parameter of type '__attribute__((__vector_size__(16 * sizeof(char)))) char' (vector of 16 'char' values) with an rvalue of type '__i8x16' (vector of 16 'int8_t' values)
> >   return (v128_t)__builtin_wasm_abs_i8x16((__i8x16)a);
> > ```
> > 
> > Even changing `char` to `signed char` doesn't work. I would change the builtins to use the better types, but the builtin definition system does not allow those more interesting types.
> > 
> > This is especially annoying because `-funsigned-char` does indeed change the behavior of the comparison intrinsics. I will have to do extra work in those functions to make it work.
> Would changing the clang definitions of the builtins to use signed char (Sc) explicitly fix this, like:
> 
> -TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", "unimplemented-simd128")
> +TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", "unimplemented-simd128")
> 
> and so on for all the wasm simd builtins?
Neat, I hadn't remembered that those signed and unsigned prefixes existed. Yes, that would enable some simplifications, but I think they would be better done in a follow-on CL.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76959/new/

https://reviews.llvm.org/D76959





More information about the cfe-commits mailing list