prusa3d / Prusa-Firmware-MMU

Other
32 stars 15 forks source link

Move MMU registers into PROGMEM #287

Closed gudnimg closed 1 year ago

gudnimg commented 1 year ago

🚧 This is not tested! But this is what I have so far. Please don't merge yet.

Change in memory: Flash: +34 bytes SRAM: -170 bytes

with 2da6f6c the code size actually shrunk by 8B (as originally expected)

Latest refactoring made the code size shrink by 44B

MMU-253

DRracer commented 1 year ago

:thinking: any idea why is it taking +34 bytes?

leptun commented 1 year ago

Just speculating here, it could be due to different optimization due to the restructured code. Loading from RAM can be done with direct addressing where the address isn't loded in any register. This is in contrast to PROGMEM where the address has to be loaded in the Z register pair. So a load from ram could be only 2 words, while a load from progmem could be 3 words, and also clobber the Z registers.

leptun commented 1 year ago

Oh, and one more critical difference is that from ram you can do a "load from ram with displacement" operation which is really useful for accessing fields in an array of struct. Loading from progmem doesn't offer this displacement mode, so the displacement might have to be computed at runtime in order to get the final pointer to progmem.

DRracer commented 1 year ago

:thinking: I just got different numbers (the ones I'd expect):

with a little tweak I got what I expected:

the only change I made:

const RegisterRec &reg = *static_cast<RegisterRec *>(pgm_read_ptr(registers + address));
gudnimg commented 1 year ago

🤔 I just got different numbers (the ones I'd expect):

  • RAM registers: 28288B
  • PROGMEM registers as in this PR: 28292B

with a little tweak I got what I expected:

  • PROGMEM registers: 28280B

the only change I made:

const RegisterRec &reg = *static_cast<RegisterRec *>(pgm_read_ptr(registers + address));

Is this correct though? Right had side is dereferencing a pointer. Doesn't &reg returning the address of reg?

DRracer commented 1 year ago

@gudnimg nope, you are right, there are more issues on the way, fixing now