-
Notifications
You must be signed in to change notification settings - Fork 44
Add code size measurement scripts #246
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
base: main
Are you sure you want to change the base?
Add code size measurement scripts #246
Conversation
scripts/get_code_size_m55.py
Outdated
| if __name__ == '__main__': | ||
| BUILD_DIR = 'build-code-size-m55' | ||
|
|
||
| build_library(BUILD_DIR, 'arm-gcc-m55.cmake') |
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.
This script feels like it's doing both too much and too little. Too much: why not take the cmake script as an argument, so you can use it for different targets/compilers? Too little: it's cross-compiling to a bare-metal target so the build fails because it's missing e.g. an entropy source: it should probably take a config as an option, with baremetal_size as the default.
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.
Ah yes it's meant to be an all-in-one demonstrator script that does everything for M55. I was planning to add ./scripts/config.py baremetal to the start but then I used it to test the code size impact of a config option (MBEDTLS_RSA_NO_CRT) and realised that would not be possible if I set the config upfront.
I'm happy to add it now though, I agree it would make this script easier to use.
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.
The changed config should be only in the build directory, not in the source directory. Otherwise the script isn't really usable outside the CI.
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.
I have added a switch --config-name allowing a config to be specified. config.py is run on a copy of the config file in the build directory that is then used. This can be implemented in a 20% more nifty way once Mbed-TLS/TF-PSA-Crypto#531 is merged.
| @@ -0,0 +1,78 @@ | |||
| #!/usr/bin/env python3 | |||
| """Examine 2 code size reports and print the difference between them. | |||
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.
What's your typical workflow with this script?
I use mbedtls-size-dwim, where my workflow is to set up one or more build directories and run e.g. mbedtls-size-dwim build-m0plus each time I commit and at any point while I'm developing. It shows the code size differences with the last commit.
With diff_code_size.py, where would I typically find the files to compare?
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.
If working in conjunction with Mbed-TLS/TF-PSA-Crypto#590 it would be something like:
# Build version A
cmake -B build-A .
cmake --build build-A --target size # or (cd build-A && make size)
# Build version B
cmake -B build-B .
cmake --build build-B --target size # or (cd build-B && make size)
# Compare code size
framework/scripts/diff_code_size.py build-A/core/code_size.json build-B/core/code_size.json
It might be easier if it found the code size report itself, should I add that feature?
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.
Ah, ok, you're typically comparing different build options? I find that I mostly want to compare different versions of the same code. That's why mbedtls-size-dwim stores sizes in a file named after the git head sha.
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.
Usually I need to compare different options but assessing the code size impact of a config option happens often enough that I value the flexibility.
This script runs the specified size command (e.g. arm-none-eabi-size) on the specified library (e.g. libtfpsacrypto.a). Signed-off-by: David Horstmann <david.horstmann@arm.com>
This script parses the JSON file we generate and pretty-prints a table of sizes along with the totals. Signed-off-by: David Horstmann <david.horstmann@arm.com>
This sets the CFLAGS correctly for Cortex-M55 and -Os. This file can be passed to CMake to allow us to easily build for M55. Signed-off-by: David Horstmann <david.horstmann@arm.com>
Add a script that compiles the library for Cortex-M55 (using the toolchain file in framework/platform) and uses other scripts to get and display the code size. Signed-off-by: David Horstmann <david.horstmann@arm.com>
This takes 2 JSON files containing the code size information and computed the difference between them. It then pretty-prints the sizes for files that are different along with the difference and percentage difference in size. Signed-off-by: David Horstmann <david.horstmann@arm.com>
d2ef087 to
69c08b2
Compare
This allows us to specify the platform version of size if required, while keeping the default as arm-none-eabi-size. Signed-off-by: David Horstmann <david.horstmann@arm.com>
Allow any toolchain file to be passed as an argument so that we can have any platform (but still use the M55 GCC toolchain file by default). Signed-off-by: David Horstmann <david.horstmann@arm.com>
Since it can now be used for any platform, it should no longer have an M55-specific name. Signed-off-by: David Horstmann <david.horstmann@arm.com>
Add a command-line argument to specify a named config to build the library in (default to baremetal_size). Do this by copying a config file from the source tree into the build tree before modification, thus preserving the source tree intact. Signed-off-by: David Horstmann <david.horstmann@arm.com>
When doing a code size diff, interpret directory names as build
directories that contain core/code_size.json.
This enables us to do:
cmake --build build-A
cmake --build build-B
framework/scripts/diff_code_size.py build-A build-B
Which is slightly more usable than having to remember the path where the
code size report is stored.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Add scripts to generate a code size report, display and diff that report.
Add a CMake toolchain file for Cortex-M55 and a script that compiles the library for M55 and measures the code size.
Why 4 scripts instead of just one?
Separating the functionality allows it to later be integrated into the build system so that we can just run
make sizein the build directory to get the code size. See Mbed-TLS/TF-PSA-Crypto#590 for that.PR checklist
Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed.