Convert bswap primitives to builtins on !x86 archs

[mhorne]

Summary

Rather than maintain our own implementations for each architecture, we can rely
on the compiler builtin to emit appropriate instructions or optimized libcalls
as available on each platform.

This conversion was done recently for x86/endian.h in e6ff6154d203.

Overall this should result in similar if not identical code generation. The
notable exception is arm, where we can now take advantage of the rev
instruction, available on armv6+. On arm64, we end up saving one instruction,
due to PR 236920.

While here, comments present in other headers were added where they were
missing. Drop the armeb remnants.

Presented as one review, although I expect to commit each file separately.

Test Plan

tinderbox passes with EXTRA_ARCHS defined.

I will smoke test on arm64/arm.

[arichardson]

Thanks, we should really avoid inline assembly if compiler intrinsics are available!
There's still quite a bit of code that's duplicated across all architectures. I think we should be able to use a the same header for all architectures since both GCC and Clang have builtin defines:

#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412

Using a mips/armeb/etc. target correctly sets #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__, so maybe it's time to trust the compiler defines instead of copy-pasting code for each architecture?

[mhorne]

In D29012#649621, @arichardson wrote:

Thanks, we should really avoid inline assembly if compiler intrinsics are available!
There's still quite a bit of code that's duplicated across all architectures. I think we should be able to use a the same header for all architectures since both GCC and Clang have builtin defines:

#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412

Using a mips/armeb/etc. target correctly sets #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__, so maybe it's time to trust the compiler defines instead of copy-pasting code for each architecture?

I agree, we can probably refine this further. The tricky bit is the places which include machine/endian.h without sys/endian.h. I'm not sure exactly how many instances of this there are, but I'm not super keen on being the one to audit them all.

Possibly, we could add a sys/_endian.h, and have each machine/endian.h become a trivial wrapper around it.

[imp]

In D29012#649628, @mhorne wrote:

In D29012#649621, @arichardson wrote:

Thanks, we should really avoid inline assembly if compiler intrinsics are available!
There's still quite a bit of code that's duplicated across all architectures. I think we should be able to use a the same header for all architectures since both GCC and Clang have builtin defines:

#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412

Using a mips/armeb/etc. target correctly sets #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__, so maybe it's time to trust the compiler defines instead of copy-pasting code for each architecture?

I agree, we can probably refine this further. The tricky bit is the places which include machine/endian.h without sys/endian.h. I'm not sure exactly how many instances of this there are, but I'm not super keen on being the one to audit them all.

some care is needed here, sys/endian.h and endian.h are externally used. Extensively.

There's also a lot of places that check for the machine/endian.h header as part of their autoconfig phase, so an exp run would be needed.

We use them in a few places in the tree. most of them would be better served with sys/endian.h, imho, but that's just me.

Possibly, we could add a sys/_endian.h, and have each machine/endian.h become a trivial wrapper around it.

The network macros are the only ones I see that might need changing, but then again, moving the be/le macros to sys/_endian.h as well would let you make them aliases for beXtoh...

We could retire a lot of cut-n-pasted code from the 90s with this one include (even w/o moving the be/le macros and having the #if for the endian for the network macros.

But it sounds like the shuffling will produce the same externally visible results, so go for it.

[mhorne]

Consolidate endian definitions to a new header sys/_endian.h, included by machine/endian.h.

[mhorne]

x86/endian.h just adds an extra level of header indirection now, so I can remove that too if it seems worthwhile. This change is already doing a lot, so I'd probably handle it as a follow-up.

[arichardson]

Great to see all this duplicated code go away! Probably best to wait with committing until someone else has also had a look since I might have missed something.

[arichardson]

sys/sys/_endian.h

38–39

I guess you could add this to be paranoid, but things will probably break anyway if you use an old compiler.
According to https://godbolt.org/z/ce9sEE this macro needs GCC 4.6

[mhorne]

@imp did you have any thoughts on this latest revision? I'll likely try to commit this tomorrow otherwise.

sys/sys/_endian.h

38–39

I thought there had been some work recently to remove provisions for building with old GCC, but I might have that confused with something else. Still, I'm not too concerned for this case since I wouldn't MFC this change past stable/13, if at all.

[imp]

I'm loving this, though I worry a tiny bit about the mips version... but my worry is fleeting if this passes universe because mips' shelf life is short, alas (I say alas because I have a fondness for the idea of supporting mips, but know that actually supporting it will be impossible once CHERI moves to arm)

sys/arm/include/endian.h

39

sweet

sys/sys/_endian.h

38–39

I did some work, but got bogged down in the mind-numbing details... :(
Bootstrapping from stable/10 or stable/9 I think might be affected by this...
... but since we don't support going that face back for C++ reasons we're fine: either they have a modern compiler from base, or as an external toolchain.

[mhorne]

In D29012#659199, @imp wrote:

I'm loving this, though I worry a tiny bit about the mips version... but my worry is fleeting if this passes universe because mips' shelf life is short, alas (I say alas because I have a fondness for the idea of supporting mips, but know that actually supporting it will be impossible once CHERI moves to arm)

I made sure to look closely at the mips bits, since I had some hesitation there too. Worse comes to worst, we can opt-out and restore any of these individual headers (but it tinderboxes fine).

Although I missed the better years for mips, I can see that being a bittersweet day when it comes.