I've implemented asin in terms of acos (which was sent to the list
a few days ago).
Tangent is implemented using a sin and a square root instead of sin(x)/cos(x).
sin(x)/cos(x) which produces much more verbose assembly than using the sqrt.
That being said, I am not sure if there's a better way to implement tan(x)
while still keeping the required precision. If someone has a better option,
I'm all ears. This implementation passes the piglit unit tests, at least.
I haven't checked if llvm.sin and llvm.cos intrinsics have enough precision for
float when used together (they didn't for just calculating sin/cos, so I figured
using both intrinsics together would just increase the error).
Uses the algorithm:
tan(x) = sin(x) / sqrt(1-sin^2(x))
An alternative is:
tan(x) = sin(x) / cos(x)
Which produces more verbose bitcode and longer assembly.
this is weird. both EG and SI have both sin and cos instructions. Is the
input normalization code so bad that we are better of doing MUL+SUB+SQRT
instead?
Uses the algorithm:
tan(x) = sin(x) / sqrt(1-sin^2(x))
An alternative is:
tan(x) = sin(x) / cos(x)
Which produces more verbose bitcode and longer assembly.
this is weird. both EG and SI have both sin and cos instructions. Is the
input normalization code so bad that we are better of doing MUL+SUB+SQRT
instead?
Those are only useful for native_sin / native_cos. For the standard function, they are far from precise enough. The current (float) sin implementation should be correct, though native_sin right now is still defined to just be the regular sin function instead of the LLVM intrinsic
>> Uses the algorithm:
>> tan(x) = sin(x) / sqrt(1-sin^2(x))
>>
>> An alternative is:
>> tan(x) = sin(x) / cos(x)
>>
>> Which produces more verbose bitcode and longer assembly.
>
> this is weird. both EG and SI have both sin and cos instructions. Is the
> input normalization code so bad that we are better of doing MUL+SUB+SQRT
> instead?
Those are only useful for native_sin / native_cos. For the standard
function, they are far from precise enough. The current (float) sin
implementation should be correct, though native_sin right now is still
defined to just be the regular sin function instead of the LLVM
intrinsic
oh I didn't know the hw implementaion was so imprecise. In that case it
makes sense. Although I wonder why it ended up needing twice as many
instructions. it looks to me that sin and cos don't differ in more than
4 operations, so CSE should have eliminated most of it.
either way it's not going to be more efficient than this patch.
just out of curiosity.
How does the precision compare to just using
atan2(x, ( sqrt(1-x^2) ) )
from 5) of your acos patch?
I assume (PI/2 -) does not shift the balance.
The precision of both implementations looks ok. The existing piglit
tests pass when tightened down to a tolerance of 1 ULP and fail at 0
ULP. Given that the spec gives us 4 ULP as allowed variance, it seems
like we're good.
I did the following, alternate implementations and did a quick check
on bitcode length and number of instructions on evergreen. It seems
like the second variation gives us sufficient precision and fewer
hardware instructions for at least the tested architecture (CEDAR on
latest svn llvm/clang).
If you prefer, I can commit the second implementation instead.
>> asin(x) = PI/2 - acos(x)
>
> LGTM.
>
> just out of curiosity.
> How does the precision compare to just using
> atan2(x, ( sqrt(1-x^2) ) )
> from 5) of your acos patch?
>
> I assume (PI/2 -) does not shift the balance.
>
The precision of both implementations looks ok. The existing piglit
tests pass when tightened down to a tolerance of 1 ULP and fail at 0
ULP. Given that the spec gives us 4 ULP as allowed variance, it seems
like we're good.
I did the following, alternate implementations and did a quick check
on bitcode length and number of instructions on evergreen. It seems
like the second variation gives us sufficient precision and fewer
hardware instructions for at least the tested architecture (CEDAR on
latest svn llvm/clang).
If you prefer, I can commit the second implementation instead.