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

Global: Change division to multiplication #26855

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

muramura
Copy link
Contributor

The STM32 has fewer clocks for multiplication than for division.

@nexton-winjeel
Copy link
Contributor

Have you checked the assembly output? There is a high chance the compiler is already making this optimisation.

@peterbarker
Copy link
Contributor

Have you checked the assembly output? There is a high chance the compiler is already making this optimisation.

We don't let the compiler do this as the results can change. I think that's -freciprocal-math.

We can choose to do it ourselves, of course, and yes, this PR does change the compiler output to do muls rather than divs.

@peterbarker peterbarker force-pushed the AP_Change_division_to_multiplication-3 branch from b480a4e to a5de365 Compare September 28, 2024 06:53
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -603,7 +603,7 @@ int decodeECU_TelemetrySlow1PacketStructure(const void* _pg_pkt, ECU_TelemetrySl

// Input voltage in Volts
// Range of voltage is 0.0f to 25.5f.
_pg_user->voltage = float32ScaledFrom1UnsignedBytes(_pg_data, &_pg_byteindex, 0.0f, 1.0f/10.0f);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a generated file, should not be modified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reverted the changes to the piccolo_protocol.

@peterbarker
Copy link
Contributor

Can be merged once the piccolocan patches are removed

@muramura muramura force-pushed the AP_Change_division_to_multiplication-3 branch from a5de365 to a0b9a90 Compare October 5, 2024 12:48
@muramura muramura force-pushed the AP_Change_division_to_multiplication-3 branch from a0b9a90 to be67cd5 Compare October 5, 2024 13:07
@peterbarker
Copy link
Contributor

@muramura please rebase

@peterbarker peterbarker self-assigned this Nov 14, 2024
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

Successfully merging this pull request may close these issues.

4 participants