Skip to content
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

RFC: Refactor to use constant expressions instead of ifdefs #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andersm
Copy link

@andersm andersm commented Jul 2, 2018

This is an RFC for an idea I mentioned at at some point, reducing the amount of ifdefs in favour of constant expressions and dead code elimination. There's some ugly compatibility dummy definitions that could maybe be abstracted away, and the string generation would need reworking to automatically create the dummy definitions, but the main point is to see if anyone else thinks this would be a good idea.

Code size comparison, tested with XC16 1.35:
BPv3: +9 bytes, BPv4: no change

@andersm andersm changed the title uart.c: Refactor to use constant expressions instead of ifdefs RFC: Refactor to use constant expressions instead of ifdefs Jul 2, 2018
@JarrettR
Copy link

JarrettR commented Jul 3, 2018

This is really good for readability

@agatti
Copy link

agatti commented Sep 17, 2018

Apologies for taking this long to check this out. I'm personally partial to #defines due to habit, although #defines should not really increase code size. Whilst for v4 boards there's plenty of ROM space to use, v3 boards can be a bit more strapped for empty space to fit things in.

@andersm can you please check if there's a way to get the compiler to not increase code size using if-blocks? In theory, if a symbol is already defined the branch block that cannot be taken should be eliminated in an early compilation pass so I wonder why you see a code size increase, after all.

@andersm
Copy link
Author

andersm commented Sep 17, 2018

I believe the code size increase comes from the case label on line 825, which is behind an #ifdef BUSPIRATEV4.

@agatti
Copy link

agatti commented Sep 17, 2018

I see. I am still not 100% sure on whether having two separate ways to split code could help, as there are quite a few #define entries that vary depending on the board type (think pin definitions). Any ideas on how to sort that out in a way that does not increase project complexity?

@andersm
Copy link
Author

andersm commented Sep 18, 2018

Defining macros for pin names is a good thing, since it abstracts away the hardware differences and makes the code easier to read.

Another strategy for reducing the number of #ifdefs is to extract the blocks into inlinable functions, separated by board type. It does mean the compiler can't check both branches of the code in one go, but can increase the readability a lot.

@agatti
Copy link

agatti commented Sep 19, 2018

The inlinable functions way sounds more like it - I've started doing something like that for the board hardware definition in c93d032, actually (still not finished yet, help is as usual welcome).

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