Adds new intrinsics for instructions that are in the final SIMD spec but did not
previously have intrinsics. Also updates the names of existing intrinsics to
reflect the final names of the underlying instructions in the spec. Keeps the
old names as deprecated functions to ease the transition to the new names.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | 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? |
General question about SIMD intrinsics: So we make dedicated wasm intrinsics only for the cases there are not general intrinsics people can use instead?
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | I was also curious about this too. | |
648 | Do we not rename the builtin to v128 as well? | |
969 | Sometimes builtin names seem slightly different from intrinsic names like this case: the intrinsic name is f32x4_ceil while the builtin name is ceil_f32x4. Is that intentional? | |
1389 | ||
1398 | What does this macro do? | |
1547 | 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. |
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | 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.) | |
648 | Yeah, I think it would be probably be more consistent to do so. | |
1389 | Oops, thanks! | |
1398 | This macro is defined when -Wdeprecated is enabled, so it can be used to detect when warnings about deprecated things should be emitted. | |
1547 | 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. |
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | 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? |
No, every instruction in the proposal has an intrinsic so users don't need to worry about learning about the compiler's built-in vector support.
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | 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. |
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
969 | Yes, the builtin names use _s and _u rather than communicating the sign in the lane interpretation and they always put the instruction prefix at the end. It's just a different arbitrary naming convention. |
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 |
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? | |
1547 | Sorry maybe I'm mistaken, but what I meant was, the function name here is But the call in the line below is |
clang/lib/Headers/wasm_simd128.h | ||
---|---|---|
171 | With a function that calls the builtin function, the builtin function is passed one of the parameters to the outer function and parameters are never integer constants (this is all resolved before inlining, I guess). When an integer constant is an argument to a macro, it gets copy-pasted to be the argument to the builtin function, so everything works out. | |
1547 | Yes, that is an intentional part of the name change, in addition to "widen" being replaced with "extend". The deprecated function with the i on the i16x8 is calling the new function with the u in both locations. |
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?