-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
esp32: IDF v5.4(v5.5-dev) compile. #16565
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -402,7 +402,7 @@ static mp_raw_code_t *load_raw_code(mp_reader_t *reader, mp_module_context_t *co | |
|
||
#if MICROPY_EMIT_MACHINE_CODE | ||
} else { | ||
const uint8_t *prelude_ptr; | ||
const uint8_t *prelude_ptr = NULL; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is unrelated to this PR, and shouldn't be needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I discovered independently this change is required to prevent an error while building against IDFv5.4 with a compiler error; perhaps a change in the compiler version included used by the IDF caused this issue to be exposed? Without this change:
EDIT: Although I agree it shouldn't be needed; reading it myself it appears correct regardless of which ifdef block is used. Not sure why the compiler disagrees. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, it looks like a compiler optimisation bug here. I'm using gcc 14.2.1 and arm-none-eabi-gcc 14.2.0 here, and neither of them complain (with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the end, are we forced to leave this fix as is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can leave this change in. It doesn't affect any other ports (the code size diff is zero for them all). |
||
#if MICROPY_EMIT_NATIVE_PRELUDE_SEPARATE_FROM_MACHINE_CODE | ||
if (kind == MP_CODE_NATIVE_PY) { | ||
// Executable code cannot be accessed byte-wise on this architecture, so copy | ||
|
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.