aheejin 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),
\
----------------
tlively wrote:
> dschuff wrote:
> > tlively wrote:
> > > 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.)
> > Ah, that makes sense. It does make me wonder, do we have any documentation
> > about those constraints? I guess this file itself is more-or-less what we
> > have, right? If the constraint is violated, is the error from the compiler
> > intelligible?
> Yes, the error should be somewhat intelligible, as far as error messages go.
> So far this is all we have, but I would like to get proper documentation at
> some point.
> 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.)
Why is a function and a macro on this? Doesn't a function also call our builtin
functions, which errors out on non-const arguments?
================
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);
----------------
tlively wrote:
> 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.
Sorry maybe I'm mistaken, but what I meant was, the function name here is
`wasm_i16x8_widen_low_u8x16`
where `i` on 16x8 and `u` on 8x16
But the call in the line below is
`return wasm_u16x8_extend_low_u8x16(__a)`
where `u` is both on 16x8 and 8x16.
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
