dda.c: use muldiv in update_current_position().

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%
This commit is contained in:
Markus Hitter 2016-11-22 14:10:16 +01:00
parent 20a0808887
commit 437cb08e42
1 changed files with 10 additions and 17 deletions

27
dda.c
View File

@ -936,17 +936,15 @@ void update_current_position() {
DDA *dda = &movebuffer[mb_tail]; DDA *dda = &movebuffer[mb_tail];
enum axis_e i; enum axis_e i;
// Use smaller values to adjust to avoid overflow in later calculations, static const axes_uint32_t PROGMEM steps_per_m_P = {
// (STEPS_PER_M_X / 1000) is a bit inaccurate for low STEPS_PER_M numbers. STEPS_PER_M_X,
static const axes_uint32_t PROGMEM steps_per_mm_P = { STEPS_PER_M_Y,
((STEPS_PER_M_X + 500) / 1000), STEPS_PER_M_Z,
((STEPS_PER_M_Y + 500) / 1000), STEPS_PER_M_E
((STEPS_PER_M_Z + 500) / 1000),
((STEPS_PER_M_E + 500) / 1000)
}; };
if (dda->live) { if (dda->live) {
uint32_t axis_steps; uint32_t axis_steps, axis_um;
for (i = X; i < AXIS_COUNT; i++) { for (i = X; i < AXIS_COUNT; i++) {
#if ! defined ACCELERATION_TEMPORAL #if ! defined ACCELERATION_TEMPORAL
@ -955,19 +953,14 @@ void update_current_position() {
#else #else
axis_steps = move_state.steps[i]; axis_steps = move_state.steps[i];
#endif #endif
axis_um = muldiv(axis_steps, 1000000, pgm_read_dword(&steps_per_m_P[i]));
current_position.axis[i] = current_position.axis[i] =
dda->endpoint.axis[i] - (int32_t)get_direction(dda, i) * dda->endpoint.axis[i] - (int32_t)get_direction(dda, i) * axis_um;
// 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]));
} }
if (dda->endpoint.e_relative) { if (dda->endpoint.e_relative) {
// We support only one extruder, so axis_steps is already valid. // We support only one extruder, so axis_um is already valid.
current_position.axis[E] = current_position.axis[E] = axis_um;
(axis_steps * 1000) / pgm_read_dword(&steps_per_mm_P[E]);
} }
// current_position.F is updated in dda_start() // current_position.F is updated in dda_start()