[kleinesfilmroellchen]

Tracking issue: rust-lang/rust#125612

On thumbv8m.main-none-eabi with -Os, this reduces memcpy from 324 bytes to 30 bytes. The assembly is so short and cute _UwU, I can include it here:

0000152a <__aeabi_memcpy>:
    152a:           1883        adds    r3, r0, r2
    152c:           4298        cmp     r0, r3
    152e:           bf28        it      cs
    1530:           4770        bxcs    lr
    1532:           b580        push    {r7, lr}
    1534:           466f        mov     r7, sp
    1536:       ,-> f811 3b01   ldrb.w  r3, [r1], #1
    153a:       |   3a01        subs    r2, #1
    153c:       |   f800 3b01   strb.w  r3, [r0], #1
    1540:       '-- d1f9        bne.n   1536 <__aeabi_memcpy+0xc>
    1542:           e8bd 4080   ldmia.w sp!, {r7, lr}
    1546:           4770        bx      lr

memset is reduced from 100 to 26 bytes at least in simple tests.

Actually using this requires optimize_for_size in alloc, like the original version of this PR. I’ll make a separate r-l/r PR once this is merged to do that.

I’ve done some testing on multiple ARM embedded targets, but overall this code should be trivially correct. The previous if <size-dependent condition> are simply turned into if false with the flag, always using the bytewise copy/write branch.

@diondokter

r? @clarfonthey

[rustbot]

⚠️ Warning ⚠️

• There are uncanonicalized issue links (such as #123) in the commit messages of the following commits.
  Please add the organization and repository before the issue number (like so rust-lang/compiler-builtins#123) to avoid issues with subtrees.
  • dfbe4cf

[rustbot]

Error: Failed to set assignee to clarfonthey: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

[kleinesfilmroellchen]

r? libs

[clarfonthey]

(note: compiler-builtins is managed by crate-maintainers, not libs. not sure if @tgross35 is the only intersection of those two, but regardless, you should just let rustbot auto-assign here)

[kleinesfilmroellchen]

r?

[rustbot]

Error: Parsing assign command in comment failed: specify user to assign to (if you want to reroll without a specific reviewer in mind, use @rustbot reroll)

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

[kleinesfilmroellchen]

@rustbot reroll

(excuse me while i misunderstand the dev guide)

[rustbot]

Error: Cannot determine a new reviewer. Use r? <username or team> to request a specific reviewer or a team.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

[kleinesfilmroellchen]

Ah okay, so that doesn’t work here. Welp.

[tgross35]

No need to request reviews on this repo really, usually it's just me.

This is a really aggressive size optimization. memcpy is called everywhere, are people really going to want to save the 300 bytes for up a 4x slowdown in other parts of code? Maybe on targets that are severely flash-limited (@Patryk27 for AVR) but I suspect most users would consider keeping the word-size copies worth it.

[diondokter]

Sometimes you really do care, yes. But this is indeed really aggressive. I wonder if it could be made in build-std that this is separately selectable instead of always on when core is built with optimize-for-size.

[kleinesfilmroellchen]

On targets I am looking at, like the CH32V003 (16K Flash) and Pebble Original with STM32F205RE (24K combined memory), this is absolutely worth it. We’ve been looking at shaving off just a kilobyte here and there, and this makes a significant difference, especially since calls to these primitives are very common.

[tgross35]

It feels like perhaps we should have levels? I wonder why this is a feature at all #t-libs > optimize_for_size.

especially since calls to these primitives are very common.

That shouldn't matter, they should be getting instantiated once.

[kleinesfilmroellchen]

Added a more aggressive additional feature, in case that’s the direction we want to go.

[clarfonthey]

Thank you!