In the light of [bugs:#3544] it is probably a good idea to print a diagnostic message of either warning or error type when the old spelling of __sdcc_external_startup with only one underscore is encountered.
Despite the better standard compliance of the new identifier, the very specific way it is used can silently break existing code, which should be avoided, i.e. made non-silent at least for a transition period.
Maybe a deliberately incompatible internal function declaration could do that job.
Maybe we need something more specific, though.
If the renaming will be a significant issue for all platforms, then I agree that getting SDCC to emit a warning when a startup function under the old name is found would be a good move. But obviously it's hard to estimate the potential impact. Perhaps a survey on the users mailing list would be in order?
With regard to the specific problem with the PDK platform and Free-PDK project headers, documentation, and examples, I think the areas of concern are two-fold:
In both cases, the user will encounter transparent (i.e. without warning or error) failure or misbehaviour due to omission of system clock initialisation and calibration that is typically done inside the startup function when using the Free-PDK headers.
In the first case, only a warning by SDCC can realistically help. In the second case, additions to the Free-PDK includes can help there, but SDCC cannot.
So, we may need a two-pronged attack to mitigate any potential problems for the PDK platform.
I guess we could temporarily (for a few SDCC releases) emit a warning when we encounter a function named _sdcc_external_startup.
I have devised a way of adding a compile-time check for the old function name to the Free-PDK headers, so I have submitted a pull request to them: https://github.com/free-pdk/pdk-includes/pull/14
That should help for the situation where new code is created using the old name, or where existing code is updated to use the latest Free-PDK headers, but is obviously not a complete solution.
Last edit: Basil Hussain 2023-02-04
Can you explain the code in your pull request?
If I read it correctly then
__sdcc_external_startupis definedHow should this warn people which do NOT define the new version
__sdcc_external_startup?Can you really check in assembly for a function definition with .ifdef? And if so, why no extra
_needs to be added like to all other function definitions coming from C ==>___sdcc_external_startup?Last edit: Free-PDK 2023-02-04
The
PDK_SET_SYSCLOCK()macro is merely a convenient point at which to make the check, seeing as it is meant to be used within the startup function in question. The check is a compile-time check - the function's code doesn't actually have to run.You have mis-interpreted the code. It checks for the old function name. SDCC, when compiling C into assembly, prefixes all function names with an underscore. Therefore, in the context of inline assembly,
_sdcc_external_startupbecomes__sdcc_external_startup(1 underscore to 2), and__sdcc_external_startupbecomes___sdcc_external_startup(2 underscores to 3).Yes. The
.ifdefdirective tests for a symbol with a defined value. A C function's entry point is an assembly label, which is usable as just another symbol whose value is equal to its location count (i.e. program address). When there is no function by the given name, no symbol exists, so the.ifdefexpression is false.Checking for functions this way is of course limited to the specific translation unit the inline assembly ends up in, so it cannot be program-wide.
The extra underscore is added, but you mis-read the code. See above.
(By the way, if we are going to discuss the GitHub pull request, please let us do it on GitHub, in the pull request's comments. It would be helpful for discussion here to remain limited to the modifications to SDCC.)
Implemented in [r13850].
Related
Commit: [r13850]