[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 16:24:24 PDT 2020
tlively added a comment.
I believe all the feedback has now been addressed.
================
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:
> 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.
================
Comment at: clang/lib/Headers/wasm_simd128.h:40
+#define __REQUIRE_CONSTANT(e) \
+ _Static_assert(__builtin_constant_p(e), "Expected constant")
+
----------------
sunfish wrote:
> In clang's xmmintrin.h, helper macros like these `__DEFAULT_FN_ATTRS` and `__REQUIRE_CONSTANT` are `#undef`ed at the end of the file.
I can do that for `__DEFAULT_FN_ATTRS` but unfortunately not for `__REQUIRE_CONSTANT` because `__REQUIRE_CONSTANT` is present in the macro expansion of `wasm_i8x16_const` and friends.
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