Consider inserting explicit casts where appropriate
Opened this issue · 4 comments
Various bits of code, such as https://github.com/amd/aocl-libm-ose/blob/aocl-3.0/src/fast/powf.c#L255, currently have implicit casts from double
to float
.
The code should likely be updated to either use the float variant of the method (such as sqrtf
in the above case) or to explicitly cast to float
so the compiler doesn't warn.
A similar scenario exists in a number of places for float
/double
to int
: https://github.com/amd/aocl-libm-ose/blob/aocl-3.0/src/fast/powf.c#L291
The code should have an explicit cast to show this conversion is intended.
For other scenarios involving more complex expressions (such as https://github.com/amd/aocl-libm-ose/blob/aocl-3.0/src/fast/powf.c#L304) it is also unclear where the cast was intended.
In the above case, the logic is: pdash = C * mdash - iw1;
.
This is effectively: uint32_t = float * int - int
, which will end up as uint32_t = (uint32_t)((float * (float)(int)) - (float)(int)
Other cases such as https://github.com/amd/aocl-libm-ose/blob/aocl-3.0/src/fast/powf.c#L314-L315 are doing:
double = int;
float = asfloat((uint32_t)double)
In particular, it isn't clear if the intent is to:
- convert to double and then downcast the double directly to float
- have used
p
(lowercase) which is anint32_t
and have that reinterpreted as afloat
- do exactly as the code is written, which is convert to a signed
double
, cast touint32_t
(which may have UB for negatives), and then reinterpret as afloat
https://github.com/amd/aocl-libm-ose/blob/aocl-3.0/src/optmized/cos.c#L230 has an implicit cast from int64_t
to int32_t
, taking only the lower 32-bits.