Skip to content
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

lwip/def.h: Simplify network byte order functions #7512

Closed
wants to merge 1 commit into from

Conversation

jjsuwa
Copy link
Contributor

@jjsuwa jjsuwa commented Aug 7, 2020

GCC built-in byte order functions

  • __builtin_bswap16()
  • __builtin_bswap32()
  • __builtin_bswap64()

are just like as:

#define ALWAYS_INLINE static __inline__ __attribute__((always_inline))

/* merely example, sub-optimal implementation */
uint32_t ALWAYS_INLINE __inlined_bswapsi2(uint32_t u) {
    return ((u & 0xFF000000) >> 24)
         | ((u & 0x00FF0000) >>  8)
         | ((u & 0x0000FF00) <<  8)
         | ((u & 0x000000FF) << 24);
}
uint64_t ALWAYS_INLINE __inlined_bswapdi2(uint64_t u) {
    return ((u & 0xFF00000000000000ULL) >> 56)
         | ((u & 0x00FF000000000000ULL) >> 40)
         | ((u & 0x0000FF0000000000ULL) >> 24)
         | ((u & 0x000000FF00000000ULL) >>  8)
         | ((u & 0x00000000FF000000ULL) <<  8)
         | ((u & 0x0000000000FF0000ULL) << 24)
         | ((u & 0x000000000000FF00ULL) << 40)
         | ((u & 0x00000000000000FFULL) << 56);
}

/* seems to be as below */
uint16_t ALWAYS_INLINE __builtin_bswap16(uint16_t x) {
    return (x >> 8) | (x << 8);
}
uint32_t ALWAYS_INLINE __builtin_bswap32(uint32_t x) {
    extern uint32_t bswapsi2(uint32_t x);
    return __builtin_constant_p(x) ? __inlined_bswapsi2(x) : bswapsi2(x);
}
uint64_t ALWAYS_INLINE __builtin_bswap64(uint64_t x) {
    extern uint64_t bswapdi2(uint64_t x);
    return __builtin_constant_p(x) ? __inlined_bswapdi2(x) : bswapdi2(x);
}
/* placed to libgcc2.c */
uint32_t __attribute__((noinline)) bswapsi2(uint32_t u) {
    return __inlined_bswapsi2(u);
}
uint64_t __attribute__((noinline)) bswapdi2(uint64_t u) {
    return __inlined_bswapdi2(u);
}

This is typical of the reinvention of the wheel.
@d-a-v
Copy link
Collaborator

d-a-v commented Aug 7, 2020

Nice proposal but not in the right place, this is an official lwIP file.
It would have to be in arch/cc.h (by #define lwip_htons ...).
but even this one is not the right one because they are both overwritten by lwip2 installer.

Real source is in lwip2 repo.
A PR would make sense there.

@jjsuwa
Copy link
Contributor Author

jjsuwa commented Sep 24, 2020

closed.

@jjsuwa jjsuwa closed this Sep 24, 2020
@jjsuwa jjsuwa deleted the builtin_bswaps branch September 24, 2020 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants