[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
mgorny added a comment. Oh, sorry that I didn't check that. Then I guess it's good enough for me, we're losing just few old Athlon 64 CPUs, I guess. I'll update the patch to remove 4.2 only. https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
dvyukov added a comment. > I don't see a big issue enabling it for x86-64 but for 32-bit systems you're > ruling out quite a lot of hardware. Tsan does not support 32-bits. https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
mgorny added a comment. In https://reviews.llvm.org/D28304#636669, @dvyukov wrote: > User -march flag won't help as runtime is prebuilt. I meant the flag used by the compiler vendor when compiler-rt is built. > SCUDO is sse4.2, but tsan is sse3. Do you actually see any problems with > sse3? It's like 10 years old. I agree with you that strictly speaking we do > wrong thing for old processors. Well, it seems that SSE3 is supported since Pentium 4 / newer versions of Athlon64. I don't see a big issue enabling it for x86-64 but for 32-bit systems you're ruling out quite a lot of hardware. My only pure x86-32 box (which I use to test stuff) does not support SSE3. > But if we disable it, we disable it for everybody. So if you don't hit > cashes with it, I would prefer to keep sse3 in tsan. Well, I guess that depends on how you build compiler-rt. On Gentoo, we just pass user CFLAGS (I need to think more on that, tbh). I would rather see the compiler vendor deciding on which CPUs are to be supported by the compiler, rather than the build system silently bumping the minimum for one component. That said, I think we need to work on improved portability of compiler runtimes shipped with clang. I will probably start an open discussion on that soonish. FWICS we pretty much 'officially' support building just one or two variants of compiler-rt for different ABIs of a single architecture. This doesn't scale well. While I'm not sure how significant the performance impact is but right now we either force compiler-rt to not use newer instructions (= make it slower) or randomly cause -march= not to be respected. https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
dvyukov added a comment. Yes, for tsan runtime detection does not make lots of sense -- we only have few SSE instructions on inlined fast path. The SSE code provides quite substantial speedup. User -march flag won't help as runtime is prebuilt. SCUDO is sse4.2, but tsan is sse3. Do you actually see any problems with sse3? It's like 10 years old. I agree with you that strictly speaking we do wrong thing for old processors. But if we disable it, we disable it for everybody. So if you don't hit cashes with it, I would prefer to keep sse3 in tsan. https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
mgorny added a comment. In https://reviews.llvm.org/D28304#635620, @cryptoad wrote: > Hey Michal, > If I understand correctly, my next move is to move the CRC32 code into it's > own file an only enable SSE 4.2 for this file? Yes, like that. Have a SSE4.2 CRC32 function in a separate file, and call it only when SSE4.2 is determined to be supported. Note that for this you'd need to refactor things a bit since it seems that the hw CRC32 code is used commonly be SSE4.2 and ARM. Furthermore, you might want to do the runtime check only when SSE4.2 was not enabled at build time, i.e. skip it if user specified -msse4.2 or -march= option enabling it explicitly. I guess you could use the `#ifdef` for that (on a file where -msse4.2 is not forced). > Shouldn't COMPILER_RT_HAS_MSSE4_2_FLAG be kept for that purpose or is there > an alternative way to do it? I don't really see a point in keeping it if you can equally easily re-add it in a later patch. https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
cryptoad added a comment. Hey Michal, If I understand correctly, my next move is to move the CRC32 code into it's own file an only enable SSE 4.2 for this file? Shouldn't COMPILER_RT_HAS_MSSE4_2_FLAG be kept for that purpose or is there an alternative way to do it? Thanks https://reviews.llvm.org/D28304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28304: [compiler-rt] [cmake] Disable appending -msse* flags implicitly
mgorny created this revision.
mgorny added reviewers: cryptoad, kcc, samsonov.
mgorny added a subscriber: cfe-commits.
Herald added subscribers: dberris, kubabrecka.
Disable the code appending -msse3 and -msse4.2 flags implicitly when
the compiler supports them. The compiler support for those flags do not
indicate that the underlying CPU will support them, and passing those
flags to the compiler may result in SSE3/SSE4.2 code being emitted
*implicitly*.
If the target platform supports SSE3/SSE4.2 appropriately, the relevant
bits should be already enabled via -march= or equivalent. In this case
passing -msse* is redundant.
If a runtime detection is desired (which seems to be a case with SCUDO),
then (as gcc manpage points out) the specific SSE4.2 needs to be
isolated into a separate file, the -msse4.2 flag can be forced only
for that file and the function defined in that file can only be called
when the CPU is determined to support SSE4.2.
This fixes SIGILL on SCUDO when it is compiled using gcc-5.4.
https://reviews.llvm.org/D28304
Files:
cmake/config-ix.cmake
lib/scudo/CMakeLists.txt
lib/tsan/CMakeLists.txt
Index: lib/tsan/CMakeLists.txt
===
--- lib/tsan/CMakeLists.txt
+++ lib/tsan/CMakeLists.txt
@@ -16,7 +16,6 @@
endif()
set(TSAN_RTL_CFLAGS ${TSAN_CFLAGS})
-append_list_if(COMPILER_RT_HAS_MSSE3_FLAG -msse3 TSAN_RTL_CFLAGS)
append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530
TSAN_RTL_CFLAGS)
append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors
Index: lib/scudo/CMakeLists.txt
===
--- lib/scudo/CMakeLists.txt
+++ lib/scudo/CMakeLists.txt
@@ -4,7 +4,6 @@
set(SCUDO_CFLAGS ${SANITIZER_COMMON_CFLAGS})
append_rtti_flag(OFF SCUDO_CFLAGS)
-append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 SCUDO_CFLAGS)
set(SCUDO_SOURCES
scudo_allocator.cpp
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -28,8 +28,6 @@
check_cxx_compiler_flag(-std=c++11 COMPILER_RT_HAS_STD_CXX11_FLAG)
check_cxx_compiler_flag(-ftls-model=initial-exec
COMPILER_RT_HAS_FTLS_MODEL_INITIAL_EXEC)
check_cxx_compiler_flag(-fno-lto COMPILER_RT_HAS_FNO_LTO_FLAG)
-check_cxx_compiler_flag("-Werror -msse3" COMPILER_RT_HAS_MSSE3_FLAG)
-check_cxx_compiler_flag("-Werror -msse4.2" COMPILER_RT_HAS_MSSE4_2_FLAG)
check_cxx_compiler_flag(--sysroot=. COMPILER_RT_HAS_SYSROOT_FLAG)
if(NOT WIN32 AND NOT CYGWIN)
Index: lib/tsan/CMakeLists.txt
===
--- lib/tsan/CMakeLists.txt
+++ lib/tsan/CMakeLists.txt
@@ -16,7 +16,6 @@
endif()
set(TSAN_RTL_CFLAGS ${TSAN_CFLAGS})
-append_list_if(COMPILER_RT_HAS_MSSE3_FLAG -msse3 TSAN_RTL_CFLAGS)
append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530
TSAN_RTL_CFLAGS)
append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors
Index: lib/scudo/CMakeLists.txt
===
--- lib/scudo/CMakeLists.txt
+++ lib/scudo/CMakeLists.txt
@@ -4,7 +4,6 @@
set(SCUDO_CFLAGS ${SANITIZER_COMMON_CFLAGS})
append_rtti_flag(OFF SCUDO_CFLAGS)
-append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 SCUDO_CFLAGS)
set(SCUDO_SOURCES
scudo_allocator.cpp
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -28,8 +28,6 @@
check_cxx_compiler_flag(-std=c++11 COMPILER_RT_HAS_STD_CXX11_FLAG)
check_cxx_compiler_flag(-ftls-model=initial-exec COMPILER_RT_HAS_FTLS_MODEL_INITIAL_EXEC)
check_cxx_compiler_flag(-fno-lto COMPILER_RT_HAS_FNO_LTO_FLAG)
-check_cxx_compiler_flag("-Werror -msse3" COMPILER_RT_HAS_MSSE3_FLAG)
-check_cxx_compiler_flag("-Werror -msse4.2" COMPILER_RT_HAS_MSSE4_2_FLAG)
check_cxx_compiler_flag(--sysroot=. COMPILER_RT_HAS_SYSROOT_FLAG)
if(NOT WIN32 AND NOT CYGWIN)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
