AdaCore / bb-runtimes

Source repository for the GNAT Bare Metal BSPs
Other
65 stars 51 forks source link

rp2040: Remove floating point aeabi symbols from Ravenscar, these are… #58

Closed JeremyGrosser closed 2 years ago

JeremyGrosser commented 2 years ago

… provided by rp2040_hal now

damaki commented 2 years ago

There's still a call to __gnat_initialize_bootrom in arm/rpi/rp2040/start-rom.S just after a reset, before clock initialisation which I think would now result in a linker error when trying to use the runtime since it's no longer implemented. As far as I can tell this initialisation is doing basically the same as the call to rp_rom_float_initialize in src/startup/crt0.S in rp2040_hal.

We need to ensure that when using rp2040_hal with the runtime that the runtime's start-rom.S calls the HAL's rp_rom_float_initialize. I think we should also ensure that the runtime builds and links with applications that don't use the HAL.

I suggest a couple minor changes:

Another think I've noticed is that in pico-sdk they define a .time_critical linker section which is used for placing code in RAM, such as the __aeabi_fadd function that jumps to the bootrom. Putting these in RAM avoids the performance hit from a XIP cache miss when performing floating point calculations. We should consider adding such a linker section to the HAL and bb-runtime's linker map with the same name so that the HAL can take advantage of it.

JeremyGrosser commented 2 years ago

Without references to the soft float table, the Initialize procedure is no longer necessary here. System.Bootrom is only supplying the __gnat_rom_func_lookup symbol now, which is needed to get a reference to the wait_for_vector procedure that runs on core 1.

I've updated the PR to remove the reference to the now non-existent __gnat_initialize_bootrom

JeremyGrosser commented 2 years ago

re: .time_critical, rp2040_hal's linker script already has a section for it, but I've been hesitant to add too many things to it... I wonder if there's a way we can make that opt-in with a config flag?

damaki commented 2 years ago

Don't you still need the rp2040_hal's bootrom to be initialised when linking with the runtime though? When building an application with bb-runtimes and rp2040_hal the HAL's bootrom won't be initialised at startup because the HAL's crt0.S won't be used. Is there another way to initialise the HAL's bootrom when using bb-runtimes?

JeremyGrosser commented 2 years ago

Oh, that's a very good point. I'll go make the changes you suggested and stop commenting before I've had coffee.

JeremyGrosser commented 2 years ago

Don't merge this yet, I think there might still be some issues with missing symbols.

Fabien-Chouteau commented 2 years ago

Hi @JeremyGrosser I will close this for now. Please re-open at any point if you want.