Add codegen for llvm cos and sin elementwise builtins
The sin and cos elementwise builtins are necessary for HLSL codegen.
Tests were added to make sure that the expected errors are encountered when these functions are given inputs of incompatible types.
The new builtins are restricted to floating point types only.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Could you update the title to make it clear this adds new Clang builtins, not intrinsics (they are lowered to LLVM intrinsics). The behavior of the new builtins should be specified in LanguageExtensions.rst (https://clang.llvm.org/docs/LanguageExtensions.html#vector-builtins)
Comment Actions
Does these support scalable vector types from ARM SVE or RISC-V? I can't remember what the rest of the __builtin_elementwise do. I ask because the backends will probably crash for them.
Comment Actions
The titled of this patch should be something like "Add builtin_elementwise_sin and builtin_elementwise_cos".
Can you explain why this uses a new builtin name instead of overloading the existing builtins to work on vectors? I can imagine reasons why, but I think it needs to be explained. I can't imagine cos and sin not being element-wise operations on a vector.
Comment Actions
It just adds additional builtins following the other vector-wise builtins.
As mentioned in https://lists.llvm.org/pipermail/cfe-dev/2021-September/068999.html, it is much easier to just use one builtin for all overloads instead of using different builtins for different overloads.
Comment Actions
Ah, I'd forgotten we had a bunch of elementwise builtins already. I wouldn't be surprised if I asked for that in that patch, actually, especially because the existing ones include some functions like max and min where you could easily imagine them being cross-lane. Nevermind.
Comment Actions
checkMathBuiltinElementType should stop scalable vector types in Sema::CheckBuiltinFunctionCall.
Comment Actions
I sat with @beanz and looked at the assembly code generated for the riscv64-unknown-elf target and the aarch64-apple-darwin target, with and without the sve target feature.
The compiler doesn't crash. The generated assembly looks good, there are 4 sinf calls, one generated for each element in the vector.
Comment Actions
Here is the code I used to test the machine code output:
typedef float float4 __attribute__((ext_vector_type(4)));
void test_builtin_elementwise_sin(float f1, float f2, double d1, double d2,
float4 vf1, float4 vf2)
{
f2 = __builtin_elementwise_sin(f1);
d2 = __builtin_elementwise_sin(d1);
vf2 = __builtin_elementwise_sin(vf1);
}f2 = builtin_elementwise_sin(f1); can be swapped for f2 = builtin_elementwise_cos(f1); to test the cos builtin,
Comment Actions
Not sure these will test scalable vector types.
Maybe something like vfloat32mf2_t or svfloat32_t?
| clang/docs/LanguageExtensions.rst | ||
|---|---|---|
| 606 ↗ | (On Diff #465236) | This list doesn't appear to be in an overall alphabetical order. It looks more like its grouped by similarity. add_sat/sub_sat are together for example. |
| clang/docs/LanguageExtensions.rst | ||
|---|---|---|
| 607 ↗ | (On Diff #465236) | As long as we're tweaking descriptions, please just call these "cosine" and "sine" instead of a cumbersome ratio description. "The cosine of x interpreted as an angle in radians" or similar. |
| clang/docs/ReleaseNotes.rst | ||
|---|---|---|
| 473 ↗ | (On Diff #465446) | Drop "llvm" from this sentence. |
Line these up with the start of the arguments on the previous line.