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

optimisation: stepper: make dda_isteps_t an array #4427

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

gudnimg
Copy link
Collaborator

@gudnimg gudnimg commented Oct 5, 2023

This way we can loop through it with a for-loop
and save some code size

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG -48 0 245934 5633 8018 2559
MK3_MULTILANG -48 0 245230 5642 8722 2550

@gudnimg gudnimg force-pushed the minor-stepper-opt branch from 28af1d6 to 3653dda Compare October 5, 2023 12:07
This way we can loop through it with a for-loop
and save some code size
@gudnimg gudnimg force-pushed the minor-stepper-opt branch from 3653dda to f6737d2 Compare October 7, 2023 22:18
@3d-gussner 3d-gussner added this to the FW 3.14.0 milestone Nov 18, 2023
@3d-gussner 3d-gussner requested review from wavexx and DRracer November 18, 2023 09:24
counter_z.lo = counter_x.lo;
counter_e.lo = counter_x.lo;
const int16_t value = -(current_block->step_event_count.lo >> 1);
for (uint8_t axis = 0; axis < NUM_AXIS; axis++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can even iterate from the back which saves 2-4 bytes more

for (uint8_t axis = NUM_AXIS-1; axis >=0 ; --axis){
    counter[axis].lo = value;
}

Caches and memory ordering is not an issue on the AVR IMHO

Copy link

@Ro3Deee Ro3Deee Nov 29, 2023

Choose a reason for hiding this comment

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

what about this, using postdecrementation :
for (uint8_t axis = NUM_AXIS; axis-- ; ){
counter[axis].lo = value;
} ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@petrubecheru Thanks but compiling with PF-build.sh the usage of Flash and RAM are the same before and after.

Copy link
Collaborator

@DRracer DRracer left a comment

Choose a reason for hiding this comment

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

🤔 most of the savings in this PR come from setting counter_* variables in loops. The rest is just a mechanical rewrite to match the proposed "array" principle.

I'm surprised that the compiler understood the intent and actually saved something while not breaking the rest of the code.

@3d-gussner 3d-gussner merged commit b37e39f into prusa3d:MK3 Nov 29, 2023
5 checks passed
@gudnimg gudnimg deleted the minor-stepper-opt branch September 1, 2024 12:04
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