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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits