amd/aocl-libm-ose

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 an int32_t and have that reinterpreted as a float
  • do exactly as the code is written, which is convert to a signed double, cast to uint32_t (which may have UB for negatives), and then reinterpret as a float

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.