tlively added inline comments.

================
Comment at: clang/lib/Headers/wasm_simd128.h:171
+
+#define wasm_v128_load8_lane(__ptr, __vec, __i)                                
\
+  ((v128_t)__builtin_wasm_load8_lane((signed char *)(__ptr), (__i8x16)(__vec), 
\
----------------
aheejin wrote:
> dschuff wrote:
> > out of curiosity, why are these macros, while all the rest (including ones 
> > that don't need declarations such as `wasm_i64x2_eq`) seem to be inline 
> > functions?
> I was also curious about this too.
The `i` parameter needs to be an integer constant, and I never figured out a 
way to enforce that for a function parameter. (But using a macro works because 
the codegen for the builtin functions can error out on non-constant arguments.)


================
Comment at: clang/lib/Headers/wasm_simd128.h:648
+static __inline__ bool __DEFAULT_FN_ATTRS wasm_v128_any_true(v128_t __a) {
+  return __builtin_wasm_any_true_i8x16((__i8x16)__a);
+}
----------------
aheejin wrote:
> Do we not rename the builtin to `v128` as well?
Yeah, I think it would be probably be more consistent to do so.


================
Comment at: clang/lib/Headers/wasm_simd128.h:1389
+
+// Old intrinsic names supported to ease transitioning to the stand names. Do
+// not use these; they will be removed in the near future.
----------------
aheejin wrote:
> 
Oops, thanks!


================
Comment at: clang/lib/Headers/wasm_simd128.h:1398
+
+#ifdef __DEPRECATED
+#define __DEPRECATED_WASM_MACRO(__name, __replacement)                         
\
----------------
aheejin wrote:
> What does this macro do?
This macro is defined when -Wdeprecated is enabled, so it can be used to detect 
when warnings about deprecated things should be emitted.


================
Comment at: clang/lib/Headers/wasm_simd128.h:1547
+static __inline__ v128_t __DEPRECATED_FN_ATTRS("wasm_u16x8_extend_low_u8x16")
+wasm_i16x8_widen_low_u8x16(v128_t __a) {
+  return wasm_u16x8_extend_low_u8x16(__a);
----------------
aheejin wrote:
> The deprecated function name and the new intrinsic say `u`. Should this be 
> `u` too? I haven't checked every single entry, but there seem to be multiple 
> instances like this.
No, the old name had an `i` here (see the change on 1059). The convention is to 
use `u` and `i` to communicate unsignedness and signedness. Previously this 
name just had a `u` on the `u8x16` at the end, which was sufficient to 
communicate whether this was the `_u` or `_s` variant of the instruction. For 
the new names, I'm trying to communicate signedness of both the parameters and 
results, so the new name uses a `u` in both locations. In contrast, e.g. 
`wasm_u16x8_narrow_i32x4` remains unchanged because the inputs to narrowing 
operations are treated as signed even if the output is unsigned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101112

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to