From 6a171724b776e55a4fdd535d78e2d1e8b6c74d74 Mon Sep 17 00:00:00 2001
From: Peter Dillinger <peterd@fb.com>
Date: Fri, 13 Sep 2019 11:04:52 -0700
Subject: [PATCH] Clean up + fix build scripts re: USE_SSE= and PORTABLE=
 (#5800)

Summary:
In preparing to utilize a new Intel instruction extension, I
noticed problems with the existing build script in regard to the
existing utilized extensions, either with USE_SSE or PORTABLE flags.

* PORTABLE=0 was interpreted the same as PORTABLE=1. Now empty and 0
mean the same. (I guess you were not supposed to set PORTABLE= if you
wanted non-portable--except that...)
* The Facebook build script extensions would set PORTABLE=1 even if
it's already set in a make var or environment. Now it does not override
a non-empty setting, so use PORTABLE=0 for fully optimized build,
overriding Facebook environment default.
* Put in an explanation of the USE_SSE flag where it's used by
build_detect_platform, and cleaned up some confusing/redundant
associated logic.
* If USE_SSE was set and expected intrinsics were not available,
build_detect_platform would exit early but build would proceed with
broken, incomplete configuration. Now warning is gracefully recovered.
* If USE_SSE was set and expected intrinsics were not available,
build would still try to use flags like -msse4.2 etc. which could lead
to unexpected compilation failure or binary incompatibility. Now those
flags are not used if the warning is issued.

This should not break or change existing, valid build scripts.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5800

Test Plan: manual case testing

Differential Revision: D17369543

Pulled By: pdillinger

fbshipit-source-id: 4ee244911680ae71144d272c40aceea548e3ce88
---
 build_tools/build_detect_platform        | 40 +++++++++++++++++-------
 build_tools/fbcode_config.sh             |  7 +++--
 build_tools/fbcode_config4.8.1.sh        |  7 +++--
 build_tools/fbcode_config_platform007.sh |  7 +++--
 4 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/build_tools/build_detect_platform b/build_tools/build_detect_platform
index 7b18a5d5f..00fa2b264 100755
--- a/build_tools/build_detect_platform
+++ b/build_tools/build_detect_platform
@@ -533,7 +533,7 @@ if test "$USE_HDFS"; then
   JAVA_LDFLAGS="$JAVA_LDFLAGS $HDFS_LDFLAGS"
 fi
 
-if test -z "$PORTABLE"; then
+if test "0$PORTABLE" -eq 0; then
   if test -n "`echo $TARGET_ARCHITECTURE | grep ^ppc64`"; then
     # Tune for this POWER processor, treating '+' models as base models
     POWER=`LD_SHOW_AUXV=1 /bin/true | grep AT_PLATFORM | grep -E -o power[0-9]+`
@@ -547,16 +547,34 @@ if test -z "$PORTABLE"; then
     COMMON_FLAGS="$COMMON_FLAGS"
   elif [ "$TARGET_OS" == "IOS" ]; then
     COMMON_FLAGS="$COMMON_FLAGS"
-  elif [ "$TARGET_OS" != "AIX" ] && [ "$TARGET_OS" != "SunOS" ]; then
+  elif [ "$TARGET_OS" == "AIX" ] || [ "$TARGET_OS" == "SunOS" ]; then
+    # TODO: Not sure why we don't use -march=native on these OSes
+    if test "$USE_SSE"; then
+      TRY_SSE_ETC="1"
+    fi
+  else
     COMMON_FLAGS="$COMMON_FLAGS -march=native "
-  elif test "$USE_SSE"; then
-    COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
   fi
-elif test "$USE_SSE"; then
-  COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
+else
+  # PORTABLE=1
+  if test "$USE_SSE"; then
+    TRY_SSE_ETC="1"
+  fi
 fi
 
-$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
+if test "$TRY_SSE_ETC"; then
+  # The USE_SSE flag now means "attempt to compile with widely-available
+  # Intel architecture extensions utilized by specific optimizations in the
+  # source code." It's a qualifier on PORTABLE=1 that means "mostly portable."
+  # It doesn't even really check that your current CPU is compatible.
+  #
+  # SSE4.2 available since nehalem, ca. 2008-2010
+  TRY_SSE42="-msse4.2"
+  # PCLMUL available since westmere, ca. 2010-2011
+  TRY_PCLMUL="-mpclmul"
+fi
+
+$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS $TRY_SSE42 -x c++ - -o /dev/null 2>/dev/null <<EOF
   #include <cstdint>
   #include <nmmintrin.h>
   int main() {
@@ -565,13 +583,12 @@ $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
   }
 EOF
 if [ "$?" = 0 ]; then
-  COMMON_FLAGS="$COMMON_FLAGS -DHAVE_SSE42"
+  COMMON_FLAGS="$COMMON_FLAGS $TRY_SSE42 -DHAVE_SSE42"
 elif test "$USE_SSE"; then
   echo "warning: USE_SSE specified but compiler could not use SSE intrinsics, disabling" >&2
-  exit 1
 fi
 
-$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
+$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS $TRY_PCLMUL -x c++ - -o /dev/null 2>/dev/null <<EOF
   #include <cstdint>
   #include <wmmintrin.h>
   int main() {
@@ -583,10 +600,9 @@ $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
   }
 EOF
 if [ "$?" = 0 ]; then
-  COMMON_FLAGS="$COMMON_FLAGS -DHAVE_PCLMUL"
+  COMMON_FLAGS="$COMMON_FLAGS $TRY_PCLMUL -DHAVE_PCLMUL"
 elif test "$USE_SSE"; then
   echo "warning: USE_SSE specified but compiler could not use PCLMUL intrinsics, disabling" >&2
-  exit 1
 fi
 
 # iOS doesn't support thread-local storage, but this check would erroneously
diff --git a/build_tools/fbcode_config.sh b/build_tools/fbcode_config.sh
index c2c39db48..4834be5f4 100644
--- a/build_tools/fbcode_config.sh
+++ b/build_tools/fbcode_config.sh
@@ -86,9 +86,10 @@ else
 fi
 CFLAGS+=" -DTBB"
 
-# use Intel SSE support for checksum calculations
-export USE_SSE=1
-export PORTABLE=1
+test "$USE_SSE" || USE_SSE=1
+export USE_SSE
+test "$PORTABLE" || PORTABLE=1
+export PORTABLE
 
 BINUTILS="$BINUTILS_BASE/bin"
 AR="$BINUTILS/ar"
diff --git a/build_tools/fbcode_config4.8.1.sh b/build_tools/fbcode_config4.8.1.sh
index 80fbdf431..5f0813a04 100644
--- a/build_tools/fbcode_config4.8.1.sh
+++ b/build_tools/fbcode_config4.8.1.sh
@@ -53,9 +53,10 @@ LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
 TBB_INCLUDE=" -isystem $TBB_BASE/include/"
 TBB_LIBS="$TBB_BASE/lib/libtbb.a"
 
-# use Intel SSE support for checksum calculations
-export USE_SSE=1
-export PORTABLE=1
+test "$USE_SSE" || USE_SSE=1
+export USE_SSE
+test "$PORTABLE" || PORTABLE=1
+export PORTABLE
 
 BINUTILS="$BINUTILS_BASE/bin"
 AR="$BINUTILS/ar"
diff --git a/build_tools/fbcode_config_platform007.sh b/build_tools/fbcode_config_platform007.sh
index 9da23fd84..51edf134f 100644
--- a/build_tools/fbcode_config_platform007.sh
+++ b/build_tools/fbcode_config_platform007.sh
@@ -86,9 +86,10 @@ else
 fi
 CFLAGS+=" -DTBB"
 
-# use Intel SSE support for checksum calculations
-export USE_SSE=1
-export PORTABLE=1
+test "$USE_SSE" || USE_SSE=1
+export USE_SSE
+test "$PORTABLE" || PORTABLE=1
+export PORTABLE
 
 BINUTILS="$BINUTILS_BASE/bin"
 AR="$BINUTILS/ar"