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

[fedor.sergeev]

Summary

Got rid of unwieldy -include Solaris.h portability solution, replacing it with interposed header and moving endian defines into Host.h.

Fixes PR28370.

[fedor.sergeev]

LLVM/Clang build passes on Sparc/X86-Solaris11/Solaris12.
libcxx cmake passes on Sparc/X86-Solaris11/Solaris12.

[krytarowski]

Do you need to modify include/llvm/module.modulemap for Solaris.h?

[fedor.sergeev]

removed reference to Solaris.h from module.modulemap, thanks to Kamil for noticing.

[fedor.sergeev]

ugh, now with a proper patch...

[joerg]

include/llvm/Support/Host.h

25

Can't you use sys/byteorder.h or is that post-Solaris11?

include/llvm/Support/Solaris/sys/regset.h

10–11

Maybe:

This file works around excessive name space pollution from the system header on Solaris hosts.

[fedor.sergeev]

Verified that sys/regset.h works by adding
+#include <sys/regset.h>
+int ES = 1;
into lib/CodeGen/LivePhysRegs.cpp source and checking that it compiles.

Btw, there is one catch here, gcc reports a pedantic warning on include_next:

In file included from  llvm/lib/CodeGen/LivePhysRegs.cpp:16:0:
llvm/include/llvm/Support/Solaris/sys/regset.h:18:2: warning: #include_next is a GCC extension
 #include_next <sys/regset.h>

I can silence this warning by adding
#pragma GCC system_header

into sys/regset.h interposing header, but I'm not sure whats the policy for using GCC pragmas in LLVM project...

[krytarowski]

include/llvm/module.modulemap

279

Do we want to install globally Solaris/sys/regset.h? Is it needed by Clang, LLDB or other projects?

[ro]

include/llvm/Support/Host.h

25

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.

[fedor.sergeev]

include/llvm/module.modulemap

279

I dont quite understand all the module implications, but Solaris/sys/regset.h is just a crude fix for system sys/regset.h.
I dont really see how it belongs to LLVM modules.

Also, currently there are no uses of regset.h outside of Solaris.h (as git grep in monorepo tells me).
So it is just a guard against possible future misuse.
It is very likely that it will never ever be used.

[krytarowski]

include/llvm/module.modulemap

279

Then we should blacklist this header from installing globally.

exclude header "Support/Solaris/sys/regset.h"

[fedor.sergeev]

include/llvm/Support/Host.h

25

sys/byteorder.h on my Solaris11/Solaris12 provides only #define _LITTLE_ENDIAN/_BIG_ENDIAN, which does not really help.

[fedor.sergeev]

module.modulemap: exclude Solaris/sys/regset.h
regset.h: modifying file comments as per Joerg's suggestion.

[fedor.sergeev]

Ahem... I'm not sure that "Support/Solaris/sys/regset.h" is a right name for this module thing.
The header is being included through -I <LLVM_MAIN_DIR>/include/llvm/Support/Solaris path, and its relative name is "sys/regset.h".

Besides, it doesnt belong to the Support module in any practical way.

To me it is more akin to normal system headers.
We should not be mentioning any system headers in this modulemap thing.

[fedor.sergeev]

removing module.modulemap entry for sys/regset.h.
For one, there is nothing specific to "module Support" in that header.

[joerg]

include/llvm/Support/Host.h

25

OK, I only have the Illumos version at hand, which uses _BIG_ENDIAN to decide whether ntohl and friends should do something or not.

25

ro: I disagree since we don't require gcc or clang to build.

[fedor.sergeev]

include/llvm/Support/Host.h

25

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...

[ro]

include/llvm/Support/Host.h

25

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.

[ro]

include/llvm/Support/Host.h

25

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.

[joerg]

include/llvm/Support/Host.h

25

The use in the Demangler is a bug. compiler-rt is different as we are not required to support arbitrary compilers for that one.

[fedor.sergeev]

a bit more portable Host.h, using _BIG_ENDIAN selector from sys/types.h

[krytarowski]

include/llvm/Support/Host.h

25

I can currently build compiler-rt only with Clang (GCC isn't supported).

[fedor.sergeev]

Perhaps, the use of __BYTE_ORDER__ in ItaniumDemangle.cpp is dictated by its declared "independence":

// It also has no dependencies on the rest of llvm. It is implemented this way
// so that it can be easily reused in libcxxabi.

I was going to adapt it to Endian.h use, and now I'm not that sure.
Anyway, that should be a separate bugfix.

I believe I'm done here :-)

[fedor.sergeev]

formatted diffs with clang-format.

[krytarowski]

I'm going to commit it now.

[fedor.sergeev]

Integrated as rL306002.

CMakeLists.txt

 if(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
   # On FreeBSD, /usr/local/* is not used by default. In order to build LLVM
   # with libxml2, iconv.h, etc., we must add /usr/local paths.
   include_directories("/usr/local/include")
   link_directories("/usr/local/lib")
 endif(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
 
 if( ${CMAKE_SYSTEM_NAME} MATCHES SunOS )
-   SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -include llvm/Support/Solaris.h")
+   # special hack for Solaris to handle crazy system sys/regset.h
+   include_directories("${LLVM_MAIN_INCLUDE_DIR}/llvm/Support/Solaris")
 endif( ${CMAKE_SYSTEM_NAME} MATCHES SunOS )
 
 # Make sure we don't get -rdynamic in every binary. For those that need it,
 # use export_executable_symbols(target).
 set(CMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS "")
 
 set(LLVM_PROFDATA_FILE "" CACHE FILEPATH
   "Profiling data file to use when compiling in order to improve runtime performance.")

include/llvm/Support/Host.h

 
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/MemoryBuffer.h"
 
 #if defined(__linux__) || defined(__GNU__) || defined(__HAIKU__)
 #include <endian.h>
 #elif defined(_AIX)
 #include <sys/machine.h>
+#elif defined(__sun)
+/* Solaris provides _BIG_ENDIAN/_LITTLE_ENDIAN selector in sys/types.h */
+#include <sys/types.h>
+#define BIG_ENDIAN 4321
+#define LITTLE_ENDIAN 1234
+#if defined(_BIG_ENDIAN)
+#define BYTE_ORDER BIG_ENDIAN
+#else
+#define BYTE_ORDER LITTLE_ENDIAN
+#endif
 #else
 #if !defined(BYTE_ORDER) && !defined(LLVM_ON_WIN32)
 #include <machine/endian.h>
 #endif
 #endif
 
 #include <string>
 

include/llvm/Support/Solaris/sys/regset.h

-/*===- llvm/Support/Solaris.h ------------------------------------*- C++ -*-===*
+/*===- llvm/Support/Solaris/sys/regset.h ------------------------*- C++ -*-===*
  *
  *                     The LLVM Compiler Infrastructure
  *
  * This file is distributed under the University of Illinois Open Source
  * License. See LICENSE.TXT for details.
  *
  *===----------------------------------------------------------------------===*
  *
- * This file contains portability fixes for Solaris hosts.
+ * This file works around excessive name space pollution from the system header
+ * on Solaris hosts.
  *
  *===----------------------------------------------------------------------===*/
 
-#ifndef LLVM_SUPPORT_SOLARIS_H
-#define LLVM_SUPPORT_SOLARIS_H
+#ifndef LLVM_SUPPORT_SOLARIS_SYS_REGSET_H
 
-#include <sys/regset.h>
-#include <sys/types.h>
-
-/* Solaris doesn't have endian.h. SPARC is the only supported big-endian ISA. */
-#define BIG_ENDIAN 4321
-#define LITTLE_ENDIAN 1234
-#if defined(__sparc) || defined(__sparc__)
-#define BYTE_ORDER BIG_ENDIAN
-#else
-#define BYTE_ORDER LITTLE_ENDIAN
-#endif
+#include_next <sys/regset.h>
 
 #undef CS
 #undef DS
 #undef ES
 #undef FS
 #undef GS
 #undef SS
 #undef EAX

include/llvm/module.modulemap

module LLVM_Utils {
   }
 
   module Support {
     requires cplusplus
 
     umbrella "Support"
     module * { export * }
 
-    // Exclude this; it's only included on Solaris.
-    exclude header "Support/Solaris.h"
-
     // Exclude this; it's fundamentally non-modular.
     exclude header "Support/PluginLoader.h"
 
     // These are intended for textual inclusion.
     textual header "Support/ARMTargetParser.def"
     textual header "Support/AArch64TargetParser.def"
   }