Skip to content

Add ability to override number of IRQ vectors #2200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

will-v-pi
Copy link
Contributor

Adds PICO_VECTOR_TABLE_NUM_IRQS define to allow overriding the number of IRQ vectors in the vector table. This is useful to reduce the size of the vector table when not using some IRQs.

For example, if no IRQs are being used then you can add this to your CMakeLists.txt to reduce the size of the vector table in the binary

target_compile_definitions(my_executable PRIVATE PICO_VECTOR_TABLE_NUM_IRQS=0)

Adds PICO_VECTOR_TABLE_NUM_IRQS to allow overriding the number of IRQ vectors in the vector table. This is useful to reduce the size of the vector table when not using some IRQs
@will-v-pi will-v-pi requested a review from kilograham January 21, 2025 17:11
@@ -52,8 +52,12 @@ __vectors:
// we don't include any IRQ vectors; we will initialize them during runtime_init in the RAM vector table
#else

#ifndef PICO_VECTOR_TABLE_NUM_IRQS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a PICO_CONFIG comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note i was going to suggest we say you cannot use the IRQ at all, which is because there will be garbage in the table which will confuse set_exclusive_irq_handler or do the wrong thing for add_shared_irq_handler

That said perhaps instead we should (as part of this fix) change runtime_init_install_ram_vector_table() to copy the provided amount of IRQs and clear the rest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change which solves this in a different way, by setting the PICO_RAM_VECTOR_TABLE_SIZE correctly based on PICO_VECTOR_TABLE_NUM_IRQS, and then checking against PICO_VECTOR_TABLE_NUM_IRQS in check_irq_param - I think this should prevent use of any IRQs which don't have vectors in the vector table?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_irq_param uses invalid_params_if which I think means that it'll become a no-op in release builds? (but I dunno if that's a problem or not)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, yeah, I think upon further thought what I actually want is a way to disable all IRQ vectors, to create a minimal vector table. I'll submit a new PR with that

@kilograham
Copy link
Contributor

think we need to be clear on what we're trying to achieve.

  1. If the goal is to minimize the stored binary size, we note that we have space for a ROM vector table most of which (though the user can provide them at build time) is full of unused slots, and could choose not to store all the vtable entries. This would allow for (with RAM vector table) use of full set of IRQs at runtime at the cost of a full size RAM vector table
  2. If the goal is to minimize both the stored binary size and the RAM usage at runtime and you don't ever plan to use IRQ number > N then we can likely clip both

Note this PR implements 2... we just need to figure if 1 is ever that interesting and if so, either do that too, or at least think how it would play with 2.

@will-v-pi
Copy link
Contributor Author

Superceded by #2233

@will-v-pi will-v-pi closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants