Skip to content

improve dev builds #324

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

Merged
merged 3 commits into from
Jul 22, 2019
Merged

improve dev builds #324

merged 3 commits into from
Jul 22, 2019

Conversation

therealprof
Copy link
Contributor

This PR basically reverts #141 and plays with inline markers for some large dev build binary size gains

@therealprof therealprof requested a review from a team as a code owner July 20, 2019 14:19
@rust-highfive
Copy link

r? @Emilgardis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Jul 20, 2019
@ryankurte ryankurte changed the title make dev builds great again improve dev builds Jul 21, 2019
@burrbull
Copy link
Member

@ryankurte can you review this PR?

@burrbull burrbull mentioned this pull request Jul 21, 2019
ryankurte
ryankurte previously approved these changes Jul 21, 2019
Copy link

@ryankurte ryankurte left a comment

Choose a reason for hiding this comment

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

lgtm!

@ryankurte
Copy link

bors r+

bors bot added a commit that referenced this pull request Jul 21, 2019
324: improve dev builds r=ryankurte a=therealprof

This PR basically reverts #141 and plays with inline markers for some large dev build binary size gains

Co-authored-by: Daniel Egger <[email protected]>
@burrbull
Copy link
Member

failed with

error: literal out of range for i32

on several targets.

#address as *const _
#[inline(always)]
pub const fn ptr() -> *const #base::RegisterBlock {
#address as _
Copy link
Member

@burrbull burrbull Jul 21, 2019

Choose a reason for hiding this comment

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

incorrect cast here (signed *i32 instead of unsigned *<width>)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is very weird. Why could a elided type result in an incompatible pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most likely the issue is not this change but the PhantomMarker change above. Anyway let's back that out.

This shaves off a couple of bytes of any dev build

Signed-off-by: Daniel Egger <[email protected]>
Turns out this @therealprof dude was wrong in blindly following general
consensus that #[inline(always)] was a bad idea. Revisiting the topic
showed that just #[inline] is not enough to get even very trivial
functions inlined in dev mode which causes a ton of bloat and a lot of
debugging fun due to many emitted extra functions without any value for
the developer.

Usual binary reductions by this change are in the area of 10-15% which
is a lot given that even a simply blinky on Cortex-M0 is several kB
already.

E.g. before:

0.5% 100.0% 4.0KiB                .text section size, the file size is 734.1KiB

after:

0.5% 100.0% 3.7KiB                .text section size, the file size is 714.5KiB

Signed-off-by: Daniel Egger <[email protected]>
@therealprof therealprof force-pushed the branch/make-dev-builds-great-again branch from 5589a6c to 3d101f9 Compare July 21, 2019 11:03
@therealprof
Copy link
Contributor Author

@ryankurte I removed the problematic change. Please have another look.

Copy link

@ryankurte ryankurte left a comment

Choose a reason for hiding this comment

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

👍

@ryankurte
Copy link

bors r+

bors bot added a commit that referenced this pull request Jul 21, 2019
324: improve dev builds r=ryankurte a=therealprof

This PR basically reverts #141 and plays with inline markers for some large dev build binary size gains

Co-authored-by: Daniel Egger <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 22, 2019

Build succeeded

@bors bors bot merged commit 3d101f9 into master Jul 22, 2019
@bors bors bot deleted the branch/make-dev-builds-great-again branch July 22, 2019 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants