debug build improvements #7591
debug build improvements #7591
Conversation
|
This change is going to expose a lot of issues. The current issue in CI is that getenv support is being built even if it is disabled in the config. Let me know if you'd like me to keep pulling on this thread, or if you'd rather just let it be. |
| @@ -160,7 +160,7 @@ CFLAGS += $(OPTIMIZATION_FLAGS) | |||
| CFLAGS += $(CFLAGS_CYW43) | |||
| #Debugging/Optimization | |||
| ifeq ($(DEBUG), 1) | |||
| CFLAGS += -ggdb3 -O3 | |||
| CFLAGS += -ggdb3 -O1 | |||
There was a problem hiding this comment.
We would usually use -Og, but you said that didn't work. if -O1 works, let's leave it and add a comment about what was tried and what didn't work.
| @@ -5,6 +5,7 @@ | |||
| #define CIRCUITPY_DIGITALIO_HAVE_INVALID_DRIVE_MODE (1) | |||
|
|
|||
| #define MICROPY_HW_LED_STATUS (&pin_CYW0) | |||
| #define MICROPY_PY_COLLECTIONS_DEQUE (1) | |||
There was a problem hiding this comment.
This should probably be promoted up to mpconfigport.h. It should be on for all but the smallest builds.
There was a problem hiding this comment.
Deque is used by wifi, and not all rp2 boards have wifi, so that's why I put it at the board level.
There was a problem hiding this comment.
See #6474; we generally want deque on; only turning it off on the smallest builds.
| #if CIRCUITPY_WIFI | ||
| #include "shared-bindings/wifi/__init__.h" | ||
| #endif | ||
|
|
||
| #if CIRCUITPY_OS_GETENV | ||
| #include "shared-module/os/__init__.h" | ||
| #endif |
There was a problem hiding this comment.
This makes sense, because supervisor/supervisor.mk only compiles this file if CIRCUITPY_WEB_WORKFLOW = 1
| #if !CIRCUITPY_WIFI | ||
| #error wifi not in build | ||
| #endif |
There was a problem hiding this comment.
I think I would rather have this kind of checking in the .mk files. If you do provide these messages, I would make them say CIRCUITPY_WIFI = 0, but this file is being included for clarity. But the corresponding C files should not be being compiled. If they are not, then there would be link errors, which is how things would be caught.
There was a problem hiding this comment.
Correct, this is just an optimization for the build to fail at compile time instead of link time, because I'm impatient waiting for builds.
A typical scenario is I'm working on some module, and I sneak in a dependency on another module by including its header file. This compiles fine, but if the other module is not in the build config I happen to be using, I will get a link error. I think it would be hard to check this in .mk files since .mk files don't know what header files C files are including.
There was a problem hiding this comment.
Are you using make -j12 etc.? I'd guess yes.
There was a problem hiding this comment.
Having #error all around the code is a +1 from me.
Linking can take 20s on x64 (-j12) and 4 minutes on my jacked (16% overclock) pi400.
|
Do we want to merge this or close? |
|
I'm fine with closing it. |
Here is a bunch of changes that I found potentially helpful while debugging issue #7333. If you think they are useful too, I can PR all of part of this. Either way it is not a big deal.