From 437cb08e42e4eb080f690007dd7abb0680e7d44b Mon Sep 17 00:00:00 2001 From: Markus Hitter Date: Tue, 22 Nov 2016 14:10:16 +0100 Subject: [PATCH] dda.c: use muldiv in update_current_position(). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently gcc doesn't manage to sort nested calculations. Putting all the muldiv()s into one line gives this error: dda.c: In function ‘update_current_position’: dda.c:969:1: error: unable to find a register to spill in class ‘POINTER_REGS’ } ^ dda.c:969:1: error: this is the insn: (insn 81 80 259 4 (set (reg:SI 82 [ D.3267 ]) (mem:SI (post_inc:HI (reg:HI 2 r2 [orig:121 ivtmp.106 ] [121])) [4 MEM[base: _97, offset: 0B]+0 S4 A8])) dda.c:952 95 {*movsi} (expr_list:REG_INC (reg:HI 2 r2 [orig:121 ivtmp.106 ] [121]) (nil))) dda.c:969: confused by earlier errors, bailing out This problem was solved by doing the calculation step by step, using intermediate variables. Glad I could help you, gcc :-) Moving performance unchanged, M114 accuracy should have improved, binary size 18 bytes bigger: ATmega sizes '168 '328(P) '644(P) '1280 Program: 19582 bytes 137% 64% 31% 16% Data: 2175 bytes 213% 107% 54% 27% EEPROM: 32 bytes 4% 2% 2% 1% --- dda.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/dda.c b/dda.c index 7824493..db3c17f 100644 --- a/dda.c +++ b/dda.c @@ -936,17 +936,15 @@ void update_current_position() { DDA *dda = &movebuffer[mb_tail]; enum axis_e i; - // Use smaller values to adjust to avoid overflow in later calculations, - // (STEPS_PER_M_X / 1000) is a bit inaccurate for low STEPS_PER_M numbers. - static const axes_uint32_t PROGMEM steps_per_mm_P = { - ((STEPS_PER_M_X + 500) / 1000), - ((STEPS_PER_M_Y + 500) / 1000), - ((STEPS_PER_M_Z + 500) / 1000), - ((STEPS_PER_M_E + 500) / 1000) + static const axes_uint32_t PROGMEM steps_per_m_P = { + STEPS_PER_M_X, + STEPS_PER_M_Y, + STEPS_PER_M_Z, + STEPS_PER_M_E }; if (dda->live) { - uint32_t axis_steps; + uint32_t axis_steps, axis_um; for (i = X; i < AXIS_COUNT; i++) { #if ! defined ACCELERATION_TEMPORAL @@ -955,19 +953,14 @@ void update_current_position() { #else axis_steps = move_state.steps[i]; #endif + axis_um = muldiv(axis_steps, 1000000, pgm_read_dword(&steps_per_m_P[i])); current_position.axis[i] = - dda->endpoint.axis[i] - (int32_t)get_direction(dda, i) * - // Should be: move_state.steps[i] * 1000000 / steps_per_m_P[i]) - // but steps[i] can be like 1000000 already, so we'd overflow. - // Unfortunately, using muldiv() overwhelms the compiler. - // Also keep the parens around this term, else results go wrong. - ((axis_steps * 1000) / pgm_read_dword(&steps_per_mm_P[i])); + dda->endpoint.axis[i] - (int32_t)get_direction(dda, i) * axis_um; } if (dda->endpoint.e_relative) { - // We support only one extruder, so axis_steps is already valid. - current_position.axis[E] = - (axis_steps * 1000) / pgm_read_dword(&steps_per_mm_P[E]); + // We support only one extruder, so axis_um is already valid. + current_position.axis[E] = axis_um; } // current_position.F is updated in dda_start()