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

Makefile.include: Generate lst file using objdump #19745

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

nandojve
Copy link
Contributor

Contribution description

The MAP file does not provide all information necessary to do a full analize of generated code. This automatically generate the LST file with all relavant C and ASM code to help inspect code generated.

Testing procedure

This was tested using AVR and Cortex-M toolchains.

@github-actions github-actions bot added the Area: build system Area: Build system label Jun 19, 2023
@nandojve nandojve requested review from maribu and benpicco June 19, 2023 19:00
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 20, 2023
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

This sure looks handy

@riot-ci
Copy link

riot-ci commented Jun 20, 2023

Murdock results

✔️ PASSED

ae61702 Makefile.include: Generate lst file using objdump

Success Failures Total Runtime
6931 0 6931 11m:28s

Artifacts

Makefile.include Outdated Show resolved Hide resolved
@nandojve nandojve force-pushed the generate_lst_file branch from 35a9470 to 25ff582 Compare June 20, 2023 14:06
Copy link
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

Ack

@nandojve
Copy link
Contributor Author

ping

@maribu
Copy link
Member

maribu commented Jun 23, 2023

bors merge

bors bot added a commit that referenced this pull request Jun 23, 2023
19745: Makefile.include: Generate lst file using objdump r=maribu a=nandojve

### Contribution description

The MAP file does not provide all information necessary to do a full analize of generated code. This automatically generate the LST file with all relavant C and ASM code to help inspect code generated.

### Testing procedure

This was tested using AVR and Cortex-M toolchains.

Co-authored-by: Gerson Fernando Budke <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 23, 2023

Build failed:

@nandojve nandojve force-pushed the generate_lst_file branch from 25ff582 to 5881cf4 Compare June 27, 2023 18:02
@nandojve
Copy link
Contributor Author

There is an issue with objdump for xtensa. It stay on a infinite loop generating the LST file.

@nandojve
Copy link
Contributor Author

Hi @maribu , can you try again if the change is fine?

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

lgtm. I assume that the objdump call can have some impact on the CI time and won't be useful there, so let's only create it in local builds.

Please squash right away and feel free to kick bors afterwards

Makefile.include Outdated Show resolved Hide resolved
The MAP file does not provide all information necessary to do a full
analize of generated code. This automatically generate the LST file
with all relavant C and ASM code to help inspect code generated.

Signed-off-by: Gerson Fernando Budke <[email protected]>
@nandojve nandojve force-pushed the generate_lst_file branch from 5881cf4 to ae61702 Compare June 29, 2023 15:30
@maribu
Copy link
Member

maribu commented Jun 29, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 29, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit c2df430 into RIOT-OS:master Jun 29, 2023
@nandojve nandojve deleted the generate_lst_file branch June 29, 2023 19:08
bors bot added a commit that referenced this pull request Jul 5, 2023
19797: Makefile.include: don't use target lstfile implicitly r=maribu a=gschorcht

### Contribution description

This PR reverts a part of PR #19745. It removes the `$(LSTFILE)` from the `BUILD_FILES` variable so that the `.lst` file isn't generated implicitly. If the `.lst` file is needed, it can be generated with `lstfile` target.

**Background**

The compilation time increased a lot with PR #19745. The reason is that the generation of the `.lst` file seems to be time-consuming. For example, on an Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz` with 8 cores and a SSD, it takes about 15 seconds for a complex application such as `tests/sys/usbus_msc` for a STM32F7 MCU.

Therefore, the compilation time for
```
BOARD=stm32f723e-disco make -j8 -C tests/sys/usbus_msc
```
increased from about 1 sec to about 16 seconds (all source files were already compiled).

Even for a small application
```
BOARD=stm32f723e-disco make -j8 -C tests/sys/shell
```
the compilation time increased from about 1 sec to about 10 seconds.

### Testing procedure

Green CI

### Issues/PRs references



Co-authored-by: Gunar Schorcht <[email protected]>
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants