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

arch: arm, arm64: Disable swi_tables.ld file when not required #68530

Conversation

rakons
Copy link
Collaborator

@rakons rakons commented Feb 5, 2024

This commit removes the need of swi_tables.ld file if the ISR table generator is not configured to use it.

Fixes #68508

@zephyrbot zephyrbot added area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture labels Feb 5, 2024
@carlescufi carlescufi added the Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. label Feb 5, 2024
arch/arm/core/swi_tables.ld Outdated Show resolved Hide resolved
arch/arm/core/swi_tables.ld Outdated Show resolved Hide resolved
@rakons rakons force-pushed the isr_disable_unused_linker_scripts branch from 9fc4528 to 66dc7a6 Compare February 5, 2024 10:13
@zephyrbot zephyrbot requested a review from dcpleung February 5, 2024 10:13
nordicjm
nordicjm previously approved these changes Feb 5, 2024
@rakons rakons force-pushed the isr_disable_unused_linker_scripts branch from 66dc7a6 to 0967179 Compare February 5, 2024 10:17
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

For other to understand and for future references, then this is imprecise:

With this prefix the build works only for Ninja and does not
work propery for other build tools.

Please include something like the following information:

Linking in Zephyr / CMake.

  • Ninja invokes linking directly from <build>.
  • Make invokes linking form <build>/zephyr.

The linker default uses cwd for looking up INCLUDE directives if not found in list of includes.
Zephyr always adds <build>/zephyr as link include using CMake,
and this is passed to ld as -L<build>/zephyr therefore using INCLUDE isr_tables_swi.ld ensures it will be correctly found in all cases.

@rakons rakons force-pushed the isr_disable_unused_linker_scripts branch from 0967179 to 616d8d8 Compare February 5, 2024 10:27
arch/arm/core/swi_tables.ld Outdated Show resolved Hide resolved
tejlmand
tejlmand previously approved these changes Feb 5, 2024
This commit removes the need of swi_tables.ld file if the
ISR table generator is not configured to use it.

Signed-off-by: Radosław Koppel <[email protected]>
@microbuilder
Copy link
Member

Please include something like the following information:

Linking in Zephyr / CMake.

  • Ninja invokes linking directly from <build>.
  • Make invokes linking form <build>/zephyr.

The linker default uses cwd for looking up INCLUDE directives if not found in list of includes.
Zephyr always adds <build>/zephyr as link include using CMake,
and this is passed to ld as -L<build>/zephyr therefore using INCLUDE isr_tables_swi.ld ensures it will be correctly found in all cases.

I made a comment in the original PR, but these changes are important enough they really need to go into the release notes. A separate PR can be raised for that, though.

This fix removes the zephyr/ prefix from linker included files.
With this prefix the build works only for Ninja and not for
other build tools.

Linking in Zephyr / CMake:
 - Ninja invokes linking directly from <build>.
 - Make invokes linking form <build>/zephyr.

The linker default uses cwd for looking up INCLUDE directives if not found
in list of includes.
Zephyr always adds <build>/zephyr as link include using CMake,
and this is passed to ld as -L<build>/zephyr therefore using
INCLUDE isr_tables_swi.ld ensures it will be correctly found in all cases.

Signed-off-by: Radosław Koppel <[email protected]>
@rakons rakons dismissed stale reviews from tejlmand, nordicjm, and henrikbrixandersen via bc1573a February 5, 2024 10:38
@rakons rakons force-pushed the isr_disable_unused_linker_scripts branch from 616d8d8 to bc1573a Compare February 5, 2024 10:38
@rakons
Copy link
Collaborator Author

rakons commented Feb 5, 2024

And fixed spaces. But please take a note that we are not consistent about defined and space after it. Personally I prefer it anyway how @pdgendt suggested - just aligned previously my style to the one found in the near surrounding.

@henrikbrixandersen henrikbrixandersen merged commit 08070fb into zephyrproject-rtos:main Feb 5, 2024
18 checks passed
@rakons
Copy link
Collaborator Author

rakons commented Feb 5, 2024

@microbuilder

I made a comment in the original PR, but these changes are important enough they really need to go into the release notes. A separate PR can be raised for that, though.

Here is proposal: #68542 68542

@rakons rakons deleted the isr_disable_unused_linker_scripts branch February 6, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Architectures area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building with "Unix Makefiles" as CMake generator no longer works
8 participants