Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider inserting explicit casts where appropriate #6

Open
tannergooding opened this issue Apr 18, 2021 · 4 comments
Open

Consider inserting explicit casts where appropriate #6

tannergooding opened this issue Apr 18, 2021 · 4 comments

Comments

@tannergooding
Copy link

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.

@tannergooding
Copy link
Author

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.

@tannergooding
Copy link
Author

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)

@tannergooding
Copy link
Author

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

@tannergooding tannergooding changed the title Consider inserting explicit casts from double to float where appropriate Consider inserting explicit casts where appropriate Apr 18, 2021
@tannergooding
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant