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

Hiffy size improvements for stm32g0 I2C #1541

Merged
merged 3 commits into from
Sep 19, 2023
Merged

Hiffy size improvements for stm32g0 I2C #1541

merged 3 commits into from
Sep 19, 2023

Conversation

cbiffle
Copy link
Collaborator

@cbiffle cbiffle commented Sep 17, 2023

This applies two changes to make Hiffy practical on the 32 kiB STM32G030, with I2C support enabled. As usual there are verbose details in the commit messages, but in summary,

  1. Made the generic "send" operations in Hiffy optional and turned them off on STM32G030. They're fairly large, at 1456 bytes. We could probably make them smaller but that's a PR for another day.
  2. Ground off 256 bytes from Hiffy's STM32G0 I2C support through refactoring and sweat.

Note that, at the time this PR was created, this was blocked on a HIF PR: oxidecomputer/hif#6

Cargo.toml Outdated Show resolved Hide resolved
@cbiffle cbiffle marked this pull request as ready for review September 19, 2023 16:18
The generic send and send-with-lease operations currently add 1456 bytes
of text, so we can either have these _or_ I2C on 32 kiB parts.

I haven't made them optional on other targets yet because, frankly,
those targets haven't needed it.
This knocks 256 bytes off I2C support (text), so we can potentially
enable it on G030.

More savings are potentially available in this code, and we've got a lot
of duplication across targets that means we can probably save space on
other machines as well.

The techniques applied here:

1. Find larger chunks of repeated code and factor them into a common
   routine (the i2c_args -> i2c_args_to_device transition)
2. Find duplicate checks for things like bounds or slicing and make them
   occur in only one place, reusing the result. (e.g. when slicing like
   buf[..n], we can assign the result to a variable and reuse it -- and
   we can combine this with the check by writing buf.get(..n).)
3. Find unhandled integer overflow cases and convert them to more
   graceful failures (currently, clients can trivially crash Hiffy by
   sending large integers for many parameters; this reduces their
   number)
4. Apply shared subexpression elimination to things like the frame
   pointer / base pointer calculation in the write routine. Previously
   they were computed separately, which didn't make it clear to the
   compiler that they're trivially related, causing it to generate
   excess checks.

I've also applied a lot more 'ok_or' and '?' patterns to remove a lot of
manual match statements on errors, under the theory that the size of the
code on the page ought to more accurately reflect its
importance/complexity in the domain (I2C). I think this change makes the
actual sources of complexity more apparent.
@cbiffle cbiffle enabled auto-merge (rebase) September 19, 2023 16:26
@cbiffle
Copy link
Collaborator Author

cbiffle commented Sep 19, 2023

@mkeeter this should be ready to go

@cbiffle cbiffle merged commit e0b57eb into master Sep 19, 2023
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants