Page MenuHomeFreeBSD
Differential D29536

libc/string/memset.c: Use unsigned long for stores
ClosedPublic
Actions

Authored by arichardson on Apr 1 2021, 2:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 9, 3:19 PM
Unknown Object (File)
Sat, Nov 8, 4:25 AM
Unknown Object (File)
Tue, Nov 4, 4:37 PM
Unknown Object (File)
Sat, Nov 1, 9:47 PM
Unknown Object (File)
Fri, Oct 31, 2:37 PM
Unknown Object (File)
Sat, Oct 25, 2:53 PM
Unknown Object (File)
Mon, Oct 20, 2:12 AM
Unknown Object (File)
Wed, Oct 15, 2:55 AM
View All Files
Subscribers
imp
jrtc27

Details

Reviewers
kib
brooks
Group Reviewers
riscv
Commits
rGfc19e3cb4fd1: libc/string/memset.c: Use unsigned long for stores
rGab147542b7c0: libc/string/memset.c: Use unsigned long for stores
Summary

While most 64-bit architectures have an assembly implementation of this
file, RISC-V does not. As we now store 8 bytes instead of 4 it should speed
up RISC-V.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 38682
Build 35571: arc lint + arc unit

Event Timeline

arichardson created this revision.Apr 1 2021, 2:38 PM
Herald added a subscriber: imp. · View Herald TranscriptApr 1 2021, 2:38 PM
arichardson requested review of this revision.Apr 1 2021, 2:38 PM
Harbormaster completed remote builds in B38253: Diff 86693.Apr 1 2021, 2:38 PM
jrtc27 added a subscriber: jrtc27.Apr 1 2021, 2:39 PM

Would (u)intptr_t make more sense so it's faster on CHERI? Doesn't make a difference upstream but does to us.

arichardson added a comment.Apr 1 2021, 2:41 PM
In D29536#662074, @jrtc27 wrote:

Would (u)intptr_t make more sense so it's faster on CHERI? Doesn't make a difference upstream but does to us.

I thought about that, but that would require additional changes to create an all-ones capability register.

jrtc27 added a comment.Apr 1 2021, 2:43 PM
In D29536#662083, @arichardson wrote:
In D29536#662074, @jrtc27 wrote:

Would (u)intptr_t make more sense so it's faster on CHERI? Doesn't make a difference upstream but does to us.

I thought about that, but that would require additional changes to create an all-ones capability register.

Oh, right, memset not bzero, never mind then

arichardson added a child revision: D28536: libthr: Avoid interposable calls to memcpy/memset/memmove.Apr 1 2021, 2:48 PM
arichardson edited the summary of this revision. (Show Details)Apr 15 2021, 10:35 AM
arichardson removed a child revision: D28536: libthr: Avoid interposable calls to memcpy/memset/memmove.

ping?

kib accepted this revision.Apr 15 2021, 11:34 AM
kib added inline comments.
lib/libc/string/memset.c
91

Did you considered lowering 3 to e.g. 2?

This revision is now accepted and ready to land.Apr 15 2021, 11:34 AM
arichardson added inline comments.Apr 15 2021, 3:14 PM
lib/libc/string/memset.c
91

This might make sense but I feel uncomfortable making that change without benchmarks.

arichardson updated this revision to Diff 87689.Apr 19 2021, 10:50 AM

Drop #undef and add a TODO comment for the threshold

This revision now requires review to proceed.Apr 19 2021, 10:50 AM
Harbormaster completed remote builds in B38680: Diff 87689.Apr 19 2021, 10:50 AM
arichardson updated this revision to Diff 87691.Apr 19 2021, 10:51 AM

Fix TODO comment

Harbormaster completed remote builds in B38682: Diff 87691.Apr 19 2021, 10:51 AM
This revision was not accepted when it landed; it landed in state Needs Review.Apr 20 2021, 12:47 AM
Closed by commit rGab147542b7c0: libc/string/memset.c: Use unsigned long for stores (authored by arichardson). · Explain Why
This revision was automatically updated to reflect the committed changes.
arichardson added a commit: rGab147542b7c0: libc/string/memset.c: Use unsigned long for stores.
arichardson added a commit: rGfc19e3cb4fd1: libc/string/memset.c: Use unsigned long for stores.May 11 2021, 10:31 AM
arichardson added a child revision: D28536: libthr: Avoid interposable calls to memcpy/memset/memmove.Jul 6 2021, 11:06 AM

Revision Contents
Changeset List

PathSize
lib/
libc/
string/
19 lines

Diff 87691

View Options

lib/libc/string/memset.c

Loading...