raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.81k stars 954 forks source link

Adding a -DRP2350 causes this error src/rp2_common/pico_crt0/embedded_start_block.inc.S:51: Error: invalid operands (*UND* and *ABS* sections) for `<<' #1974

Closed TerryFogg closed 1 week ago

TerryFogg commented 2 months ago

Simply adding a compile time definition -DRP2350 to a pico2 CMake project and compiling the crt0.S with the GNU assembler causes the following error from the included file embedded_start_block.inc.S "pico-sdk/src/rp2_common/pico_crt0/embedded_start_block.inc.S:51: Error: invalid operands (UND and ABS sections) for `<<'"

A simple #undef RP2350 placed at the beginning of "embedded_start_block.inc.S" allows it to compile/assemble properly

lurch commented 1 month ago

A simple #undef RP2350 placed at the beginning of "embedded_start_block.inc.S" allows it to compile/assemble properly

That might break e.g. https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_crt0/embedded_start_block.inc.S#L54 ?

Simply adding a compile time definition -DRP2350 to a pico2 CMake project

I think the expectation is that you'd pass -DPICO_BOARD=pico2 (and/or -DPICO_PLATFORM=rp2350, see https://github.com/raspberrypi/pico-sdk/releases/tag/2.0.0 for more info). If you want to check in your own code whether you're building for RP2040 or RP2350, the SDK mainly uses #if PICO_RP2040 and #if !PICO_RP2040.

kilograham commented 1 month ago

That might break e.g. https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_crt0/embedded_start_block.inc.S#L54 ?

His fix is because of that line

TerryFogg commented 1 month ago

embedded_start_block.inc.S uses the following line 48: PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2040) | \

line 54: PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2350) | \

For a RP2040 project and an addition of a compile definition of -DRP2040, the project compiles okay (Silently wrong)

For a RP2350 project and an addition of a compile definition of -DRP2350, the project fails and uncovers a problem which I didn't know was there.

PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2350) Maybe using which PICOBIN_IMAGE_TYPE_EXE_CHIP_RP2040 and PICOBIN_IMAGE_TYPE_EXE_CHIP_RP2350 would be a better solution

lurch commented 1 month ago

Ahhh, I've just found https://github.com/raspberrypi/pico-sdk/blob/develop/src/common/boot_picobin_headers/include/boot/picobin.h which explains everything... (I should have done more research before posting my previous comment, sorry!)

#define PICOBIN_INDEX_TO_BITS(y, x) (y ## _ ## x << y ## _LSB)

#define PICOBIN_IMAGE_TYPE_EXE_CHIP_LSB              _u(12)
#define PICOBIN_IMAGE_TYPE_EXE_CHIP_BITS             _u(0x7000)
#define PICOBIN_IMAGE_TYPE_EXE_CHIP_RP2040           _u(0)
#define PICOBIN_IMAGE_TYPE_EXE_CHIP_RP2350           _u(1)
#define PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(x) PICOBIN_INDEX_TO_BITS(PICOBIN_IMAGE_TYPE_EXE_CHIP, x)

I believe this means that (in the case where you haven't done -DRP2040) PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2040) expands to PICOBIN_INDEX_TO_BITS(PICOBIN_IMAGE_TYPE_EXE_CHIP, RP2040) which in turn expands to (PICOBIN_IMAGE_TYPE_EXE_CHIP ## _ ## RP2040 << PICOBIN_IMAGE_TYPE_EXE_CHIP ## _LSB) which then becomes (PICOBIN_IMAGE_TYPE_EXE_CHIP_RP2040 << PICOBIN_IMAGE_TYPE_EXE_CHIP_LSB), which finally evaluates to (_u(0) << _u(12)) i.e. 0. Whereas PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2350) evaluates to (_u(1) << _u(12)) i.e. 4096. Aren't preprocessor macros fun? :wink:

Whereas if you have done -DRP2350 I'm guessing this means PICOBIN_IMAGE_TYPE_EXE_CHIP_AS_BITS(RP2350) expands to PICOBIN_INDEX_TO_BITS(PICOBIN_IMAGE_TYPE_EXE_CHIP, ) which then tries to expand to (PICOBIN_IMAGE_TYPE_EXE_CHIP ## _ ## << PICOBIN_IMAGE_TYPE_EXE_CHIP ## _LSB) which then fails when it tries to do (PICOBIN_IMAGE_TYPE_EXE_CHIP_<< PICOBIN_IMAGE_TYPE_EXE_CHIP_LSB) because PICOBIN_IMAGE_TYPE_EXE_CHIP_ isn't a valid symbol. And if my reasoning is correct, I guess this means that you'd also get similar errors if you did -DARM or -DRISCV ?

Yes, this doesn't address your original problem; I'm just providing extra background-info in case it's useful.

kilograham commented 1 week ago

merged into develop