[PATCH] D34136: [Solaris] replace Solaris.h hack with a set of better hacks

[Joerg Sonnenberger via Phabricator via llvm-commits]

joerg added inline comments.


================
Comment at: include/llvm/Support/Host.h:25
+#elif defined(__sun)
+/* Solaris doesn't have endian.h. SPARC is the only supported big-endian ISA. */
+#define BIG_ENDIAN 4321
----------------
ro wrote:
> ro wrote:
> > fedor.sergeev wrote:
> > > joerg wrote:
> > > > joerg wrote:
> > > > > fedor.sergeev wrote:
> > > > > > ro wrote:
> > > > > > > joerg wrote:
> > > > > > > > Can't you use sys/byteorder.h or is that post-Solaris11?
> > > > > > > Wouldn't it be better to just switch LLVM to always use
> > > > > > > __BYTE_ORDER__, __ORDER_LITTLE_ENDIAN__, __ORDER_BIG_ENDIAN__ which are predefined by
> > > > > > > both clang and gcc?  There are only a few files left using
> > > > > > > BYTE_ORDER and friends.
> > > > > > sys/byteorder.h on my Solaris11/Solaris12 provides only #define _LITTLE_ENDIAN/_BIG_ENDIAN, which does not really help.
> > > > > > 
> > > > > OK, I only have the Illumos version at hand, which uses _BIG_ENDIAN to decide whether ntohl and friends should do something or not.
> > > > ro: I disagree since we don't require gcc or clang to build.
> > > You can use _BIG_ENDIAN instead of __sparc, thats all. You still have to manually define all those 1234 values.
> > > And, btw, the header providing this define is sys/isa_defs.h, not sys/byteorder.h.
> > > I wonder how stable this interface is...
> > joerg: Even so, there are already unconditional uses of
> > __BYTE_ORDER__ etc. inside LLVM without guards or
> > a fallback definition (e.g. lib/Demangle/ItaniumDemangle.cpp.
> > projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc), so I though this might
> > be a way forward.
> According to types.h(3HEAD), <sys/types.h> is the header
> required to get the definitions of _BIG_ENDIAN and
> _LITTLE_ENDIAN.  <sys/isa_defs.h> isn't mentioned
> anywhere, so seems like an implementation detail.
The use in the Demangler is a bug. compiler-rt is different as we are not required to support arbitrary compilers for that one.


https://reviews.llvm.org/D34136