swab.c(libc): use a simplified version of byte swapping #1086
Conversation
|
This seems like a fine cleanup. With this change there is no copyrightable content left so I'd replace the license block entirely, ideally with a 2-clause BSD or MIT license.
The size is never known within the translation unit of swab.c so short of using LTO, the compiler's ability to optimize it is zero, but this "optimization" doesn't seems to make much sense on a modern processor. |
Thanks for the brief explanation! I've updated the license information. However, I also added "copyright by rilysh" part (in OpenBSD swab, they also seem added this), but if it isn't required, I'll remove it too!
I agree, I forgot (didn't notice) that the call would be from a different source unit...sorry for the confusion. |
| @@ -1,58 +1,44 @@ | |||
| /*- | |||
lib/libc/string/swab.c
Outdated
| * | ||
| * This code is derived from software contributed to Berkeley by | ||
| * Jeffrey Mogul. | ||
| * Copyright (c) 2024 rilysh <nightquick@proton.me> |
There was a problem hiding this comment.
With SPDX, we've preferred to omit the actual license text where possible.
This looks like a good case, no?
There was a problem hiding this comment.
Yeah, sounds reasonable, I've updated it.
|
Looking very close. A couple minor nits on the license. If you change, could you also fold these two commits together? Thanks! |
This version of swab function simplifies the logic of swapping adjacent bytes. Previous version of swab() used an arbitrary unrolling, which was relevant back in the day but unnecessary for modern compilers, as if the input size is known at compile time, they can do it automatically. This version of swab() is inspired by musl. A similar version can be found at: https://github.com/openbsd/src/blob/master/lib/libc/string/swab.c Signed-off-by: rilysh <nightquick@proton.me>
Signed-off-by: rilysh <nightquick@proton.me>
|
Style checker seems didn't run (https://github.com/freebsd/freebsd-src/actions/runs/7758887131/job/21161673147?pr=1086) here, hmm, but seems unrelated... |
|
I think this is ready, but it may still bounce due to style (I'll re-run it before I land, but the change looks good so far). |
|
pushed |
This version of swab function simplifies the logic of swapping adjacent bytes. Previous version of swab() used an arbitrary unrolling, which was relevant back in the day but unnecessary for modern compilers, as if the input size is known at compile time, they can do it automatically. This version of swab() is inspired by musl. A similar version can be found at: https://github.com/openbsd/src/blob/master/lib/libc/string/swab.c Signed-off-by: rilysh <nightquick@proton.me> Reviewed by: imp Pull Request: #1086
This version of swab function simplifies the logic of swapping adjacent bytes. Previous version of swab() used an arbitrary unrolling, which was relevant back in the day but unnecessary for modern compilers, as if the input size is known at compile time, they can do it automatically. This version of swab() is inspired by musl. A similar version can be found at: https://github.com/openbsd/src/blob/master/lib/libc/string/swab.c Signed-off-by: rilysh <nightquick@proton.me> Reviewed by: imp Pull Request: freebsd/freebsd-src#1086
This version of swab function simplifies the logic of swapping adjacent bytes. Previous version of swab() used an arbitrary unrolling, which was relevant back in the day but unnecessary for modern compilers, as if the input size is known at compile time, they can do it automatically.
This version of swab() is inspired by musl.
A similar version can be found at: https://github.com/openbsd/src/blob/master/lib/libc/string/swab.c
Godbolt testing: https://godbolt.org/z/8TMxx1jos
Note: I didn't remove the "derived from ..." section. I'm not sure whether this would be necessary to do, and if so, please let me know!