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

scripts: build: gen_isr_tables: Cleanup access to ".intList" #63282

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

rakons
Copy link
Collaborator

@rakons rakons commented Sep 29, 2023

This commit removes the needs or generating isrList.bin temporary file. Now gen_isr_tables.py script access the required section directly in elf file, that was accessed by the script anyway.
It simplifies the building removing one step.

This is the part of #60838 PR that should not rise any controversy.
Just simplification of the build process without changing the way the intList section is handled later.

@tejlmand
Copy link
Collaborator

@evgeniy-paltsev could you please check that this works as expected with ARC mwdt.

$<TARGET_PROPERTY:bintools,elfconvert_flag_infile>$<TARGET_FILE:${ZEPHYR_LINK_STAGE_EXECUTABLE}>
$<TARGET_PROPERTY:bintools,elfconvert_flag_outfile>isrList.bin
$<TARGET_PROPERTY:bintools,elfconvert_flag_final>
OUTPUT isr_tables.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

proposed change fails with armclang:

$ ninja -C build 
ninja: Entering directory `build'
[1/134] Preparing syscall dependency handling

[2/134] Generating include/generated/version.h
-- Zephyr version: 3.4.99 (/projects/github/ncs/zephyr), build: zephyr-v3.4.0-4679-g2327fc53e19d
[131/134] Generating isr_tables.c
FAILED: zephyr/isr_tables.c /projects/github/ncs/zephyr/build/zephyr/isr_tables.c 
cd /projects/github/ncs/zephyr/build/zephyr && /usr/bin/python3 /projects/github/ncs/zephyr/scripts/build/gen_isr_tables.py --output-source isr_tables.c --kernel /projects/github/ncs/zephyr/build/zephyr/zephyr_pre0.elf --sw-isr-table --vector-table
gen_isr_tables.py: error: Empty ".intList" section!

ninja: build stopped: subcommand failed.

Copy link
Collaborator Author

@rakons rakons Oct 3, 2023

Choose a reason for hiding this comment

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

The generated linker_zephyr_pre0.cmd file for armclang contains:

IDT_LIST 0x20040000 NOCOMPRESS 0x800
{

  intList 0x20040000
  {
    *.o(.irq_info, +First)
    *.o(.intList)
  }
}

The section name is without a dot, it should be ".intList".
Fix proposals:

  • Update linker script to contain a do in intList section name
  • Update the python script to accept ".intList" or "intList" section names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have to go with the second answer. The section names preceded with dots should not be used as user sections. They are not strictly forbidden but they should be avoided and they are making trouble for clang. So updating clang scripts it not an option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@evgeniy-paltsev
Copy link
Collaborator

@tejlmand it works fine with ARC MWDT, thanks!

tejlmand
tejlmand previously approved these changes Oct 4, 2023
This commit removes the needs or generating isrList.bin temporary file.
Now gen_isr_tables.py script access the required section directly in
elf file, that was accessed by the script anyway.
It simplifies the building removing one step.

Signed-off-by: Radosław Koppel <radoslaw.koppel@nordicsemi.no>
@rakons
Copy link
Collaborator Author

rakons commented Oct 5, 2023

@evgeniy-paltsev - We managed to make armclang working - are we good now to get acceptation and continue?

Copy link
Collaborator

@carlocaione carlocaione left a comment

Choose a reason for hiding this comment

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

but @evgeniy-paltsev needs to take a look

@evgeniy-paltsev
Copy link
Collaborator

@carlocaione let me test it again, just in case

@fabiobaltieri fabiobaltieri added this to the v3.6.0 milestone Oct 6, 2023
@carlescufi carlescufi merged commit 48b93ee into zephyrproject-rtos:main Oct 20, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants