Skip to content

Commit 6a17172

Browse files
pdillingerfacebook-github-bot
authored andcommitted
Clean up + fix build scripts re: USE_SSE= and PORTABLE= (facebook#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: facebook#5800 Test Plan: manual case testing Differential Revision: D17369543 Pulled By: pdillinger fbshipit-source-id: 4ee244911680ae71144d272c40aceea548e3ce88
1 parent 9ba88a1 commit 6a17172

File tree

4 files changed

+40
-21
lines changed

4 files changed

+40
-21
lines changed

build_tools/build_detect_platform

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ if test "$USE_HDFS"; then
533533
JAVA_LDFLAGS="$JAVA_LDFLAGS $HDFS_LDFLAGS"
534534
fi
535535

536-
if test -z "$PORTABLE"; then
536+
if test "0$PORTABLE" -eq 0; then
537537
if test -n "`echo $TARGET_ARCHITECTURE | grep ^ppc64`"; then
538538
# Tune for this POWER processor, treating '+' models as base models
539539
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
547547
COMMON_FLAGS="$COMMON_FLAGS"
548548
elif [ "$TARGET_OS" == "IOS" ]; then
549549
COMMON_FLAGS="$COMMON_FLAGS"
550-
elif [ "$TARGET_OS" != "AIX" ] && [ "$TARGET_OS" != "SunOS" ]; then
550+
elif [ "$TARGET_OS" == "AIX" ] || [ "$TARGET_OS" == "SunOS" ]; then
551+
# TODO: Not sure why we don't use -march=native on these OSes
552+
if test "$USE_SSE"; then
553+
TRY_SSE_ETC="1"
554+
fi
555+
else
551556
COMMON_FLAGS="$COMMON_FLAGS -march=native "
552-
elif test "$USE_SSE"; then
553-
COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
554557
fi
555-
elif test "$USE_SSE"; then
556-
COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
558+
else
559+
# PORTABLE=1
560+
if test "$USE_SSE"; then
561+
TRY_SSE_ETC="1"
562+
fi
557563
fi
558564

559-
$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
565+
if test "$TRY_SSE_ETC"; then
566+
# The USE_SSE flag now means "attempt to compile with widely-available
567+
# Intel architecture extensions utilized by specific optimizations in the
568+
# source code." It's a qualifier on PORTABLE=1 that means "mostly portable."
569+
# It doesn't even really check that your current CPU is compatible.
570+
#
571+
# SSE4.2 available since nehalem, ca. 2008-2010
572+
TRY_SSE42="-msse4.2"
573+
# PCLMUL available since westmere, ca. 2010-2011
574+
TRY_PCLMUL="-mpclmul"
575+
fi
576+
577+
$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS $TRY_SSE42 -x c++ - -o /dev/null 2>/dev/null <<EOF
560578
#include <cstdint>
561579
#include <nmmintrin.h>
562580
int main() {
@@ -565,13 +583,12 @@ $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
565583
}
566584
EOF
567585
if [ "$?" = 0 ]; then
568-
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_SSE42"
586+
COMMON_FLAGS="$COMMON_FLAGS $TRY_SSE42 -DHAVE_SSE42"
569587
elif test "$USE_SSE"; then
570588
echo "warning: USE_SSE specified but compiler could not use SSE intrinsics, disabling" >&2
571-
exit 1
572589
fi
573590

574-
$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
591+
$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS $TRY_PCLMUL -x c++ - -o /dev/null 2>/dev/null <<EOF
575592
#include <cstdint>
576593
#include <wmmintrin.h>
577594
int main() {
@@ -583,10 +600,9 @@ $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
583600
}
584601
EOF
585602
if [ "$?" = 0 ]; then
586-
COMMON_FLAGS="$COMMON_FLAGS -DHAVE_PCLMUL"
603+
COMMON_FLAGS="$COMMON_FLAGS $TRY_PCLMUL -DHAVE_PCLMUL"
587604
elif test "$USE_SSE"; then
588605
echo "warning: USE_SSE specified but compiler could not use PCLMUL intrinsics, disabling" >&2
589-
exit 1
590606
fi
591607

592608
# iOS doesn't support thread-local storage, but this check would erroneously

build_tools/fbcode_config.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,10 @@ else
8686
fi
8787
CFLAGS+=" -DTBB"
8888

89-
# use Intel SSE support for checksum calculations
90-
export USE_SSE=1
91-
export PORTABLE=1
89+
test "$USE_SSE" || USE_SSE=1
90+
export USE_SSE
91+
test "$PORTABLE" || PORTABLE=1
92+
export PORTABLE
9293

9394
BINUTILS="$BINUTILS_BASE/bin"
9495
AR="$BINUTILS/ar"

build_tools/fbcode_config4.8.1.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
5353
TBB_INCLUDE=" -isystem $TBB_BASE/include/"
5454
TBB_LIBS="$TBB_BASE/lib/libtbb.a"
5555

56-
# use Intel SSE support for checksum calculations
57-
export USE_SSE=1
58-
export PORTABLE=1
56+
test "$USE_SSE" || USE_SSE=1
57+
export USE_SSE
58+
test "$PORTABLE" || PORTABLE=1
59+
export PORTABLE
5960

6061
BINUTILS="$BINUTILS_BASE/bin"
6162
AR="$BINUTILS/ar"

build_tools/fbcode_config_platform007.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,10 @@ else
8686
fi
8787
CFLAGS+=" -DTBB"
8888

89-
# use Intel SSE support for checksum calculations
90-
export USE_SSE=1
91-
export PORTABLE=1
89+
test "$USE_SSE" || USE_SSE=1
90+
export USE_SSE
91+
test "$PORTABLE" || PORTABLE=1
92+
export PORTABLE
9293

9394
BINUTILS="$BINUTILS_BASE/bin"
9495
AR="$BINUTILS/ar"

0 commit comments

Comments
 (0)