[builtins] Prevent duplicate definitions for overridden functions

[fjricci]

Summary

Some architecture-specific function overrides (for example, i386/ashrdi3.S)
duplicate generic functions (in that case, ashrdi3.c). Prevent duplicate definitions
by filtering out the generic files before compiling.

[compnerd]

cmake/Modules/CompilerRTUtils.cmake

290 ↗

(On Diff #112743)

Seems like you are re-implementing darwin_filter_builtin_sources. Can you rename and reuse that instead?

[fjricci]

Re-use existing darwin function

[compnerd]

LGTM! Thanks for the follow up cleanup work :-). Id say give @beanz a day or so to look at it, but this is mostly just a function renaming, and the new codepaths are okay.

[fjricci]

Thanks. Verified that this patch doesn't cause any files to be rebuilt when targeting macOS.

[thakis]

compiler-rt/trunk/lib/builtins/CMakeLists.txt

500

Didn't this do the same thing already? What did this down here not catch?

D36555, which went out to review before this change but landed later, added x86_ARCH_SOURCES which contains floatundixf.c which has a specialized implementation. x86_ARCH_SOURCES currently doesn't go through filter_builtin_sources() (due to it not yet being in the tree when this patch landed), but due to this loop down here it appears only once anyways.

So now I'm confused what exactly the effect of this patch here was.

compiler-rt/trunk/cmake/Modules/CompilerRTDarwinUtils.cmake

if(LIB_DEPENDS AND LIB_LIPO_FLAGS)
     add_dependencies(${LIB_PARENT_TARGET} ${name})
     install(FILES ${LIB_OUTPUT_DIR}/lib${name}.a
       DESTINATION ${LIB_INSTALL_DIR})
   else()
     message(WARNING "Not generating lipo target for ${name} because no input libraries exist.")
   endif()
 endfunction()
 
-# Filter out generic versions of routines that are re-implemented in
-# architecture specific manner.  This prevents multiple definitions of the
-# same symbols, making the symbol selection non-deterministic.
-function(darwin_filter_builtin_sources output_var exclude_or_include excluded_list)
-  if(exclude_or_include STREQUAL "EXCLUDE")
-    set(filter_action GREATER)
-    set(filter_value -1)
-  elseif(exclude_or_include STREQUAL "INCLUDE")
-    set(filter_action LESS)
-    set(filter_value 0)
-  else()
-    message(FATAL_ERROR "darwin_filter_builtin_sources called without EXCLUDE|INCLUDE")
-  endif()
-
-  set(intermediate ${ARGN})
-  foreach (_file ${intermediate})
-    get_filename_component(_name_we ${_file} NAME_WE)
-    list(FIND ${excluded_list} ${_name_we} _found)
-    if(_found ${filter_action} ${filter_value})
-      list(REMOVE_ITEM intermediate ${_file})
-    elseif(${_file} MATCHES ".*/.*\\.S" OR ${_file} MATCHES ".*/.*\\.c")
-      get_filename_component(_name ${_file} NAME)
-      string(REPLACE ".S" ".c" _cname "${_name}")
-      list(REMOVE_ITEM intermediate ${_cname})
-    endif ()
-  endforeach ()
-  set(${output_var} ${intermediate} PARENT_SCOPE)
-endfunction()
-
 # Generates builtin libraries for all operating systems specified in ARGN. Each
 # OS library is constructed by lipo-ing together single-architecture libraries.
 macro(darwin_add_builtin_libraries)
   set(DARWIN_EXCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/Darwin-excludes)
 
   set(CFLAGS "-fPIC -O3 -fvisibility=hidden -DVISIBILITY_HIDDEN -Wall -fomit-frame-pointer")
   set(CMAKE_C_FLAGS "")
   set(CMAKE_CXX_FLAGS "")
   set(CMAKE_ASM_FLAGS "")
 
   set(PROFILE_SOURCES ../profile/InstrProfiling 
                       ../profile/InstrProfilingBuffer
                       ../profile/InstrProfilingPlatformDarwin
                       ../profile/InstrProfilingWriter)
   foreach (os ${ARGN})
     list_intersect(DARWIN_BUILTIN_ARCHS DARWIN_${os}_ARCHS BUILTIN_SUPPORTED_ARCH)
     foreach (arch ${DARWIN_BUILTIN_ARCHS})
       darwin_find_excluded_builtins_list(${arch}_${os}_EXCLUDED_BUILTINS
                               OS ${os}
                               ARCH ${arch}
                               MIN_VERSION ${DARWIN_${os}_BUILTIN_MIN_VER})
 
-      darwin_filter_builtin_sources(filtered_sources
+      filter_builtin_sources(filtered_sources
         EXCLUDE ${arch}_${os}_EXCLUDED_BUILTINS
         ${${arch}_SOURCES})
 
       darwin_add_builtin_library(clang_rt builtins
                               OS ${os}
                               ARCH ${arch}
                               SOURCES ${filtered_sources}
                               CFLAGS ${CFLAGS} -arch ${arch}
                               PARENT_TARGET builtins)
     endforeach()
 
     # Don't build cc_kext libraries for simulator platforms
     if(NOT DARWIN_${os}_SKIP_CC_KEXT)
       foreach (arch ${DARWIN_BUILTIN_ARCHS})
         # By not specifying MIN_VERSION this only reads the OS and OS-arch lists.
         # We don't want to filter out the builtins that are present in libSystem
         # because kexts can't link libSystem.
         darwin_find_excluded_builtins_list(${arch}_${os}_EXCLUDED_BUILTINS
                               OS ${os}
                               ARCH ${arch})
 
-        darwin_filter_builtin_sources(filtered_sources
+        filter_builtin_sources(filtered_sources
           EXCLUDE ${arch}_${os}_EXCLUDED_BUILTINS
           ${${arch}_SOURCES})
 
         # In addition to the builtins cc_kext includes some profile sources
         darwin_add_builtin_library(clang_rt cc_kext
                                 OS ${os}
                                 ARCH ${arch}
                                 SOURCES ${filtered_sources} ${PROFILE_SOURCES}
if(COMPILER_RT_SUPPORTED_ARCH MATCHES ".*armv.*")
     set(armv6m_FUNCTIONS ${common_FUNCTIONS} ${arm_FUNCTIONS})
     set(armv7m_FUNCTIONS ${common_FUNCTIONS} ${arm_FUNCTIONS} ${thumb2_FUNCTIONS})
     set(armv7em_FUNCTIONS ${common_FUNCTIONS} ${arm_FUNCTIONS} ${thumb2_FUNCTIONS})
     set(armv7_FUNCTIONS ${common_FUNCTIONS} ${arm_FUNCTIONS} ${thumb2_FUNCTIONS} ${thumb2_64_FUNCTIONS})
     set(i386_FUNCTIONS ${common_FUNCTIONS} ${i386_FUNCTIONS})
     set(x86_64_FUNCTIONS ${common_FUNCTIONS})
 
     foreach(arch ${DARWIN_macho_embedded_ARCHS})
-      darwin_filter_builtin_sources(${arch}_filtered_sources
+      filter_builtin_sources(${arch}_filtered_sources
         INCLUDE ${arch}_FUNCTIONS
         ${${arch}_SOURCES})
       if(NOT ${arch}_filtered_sources)
         message("${arch}_SOURCES: ${${arch}_SOURCES}")
         message("${arch}_FUNCTIONS: ${${arch}_FUNCTIONS}")
         message(FATAL_ERROR "Empty filtered sources!")
       endif()
     endforeach()

compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake

macro(construct_compiler_rt_default_triple)
   # Determine if test target triple is specified explicitly, and doesn't match the
   # default.
   if(NOT COMPILER_RT_DEFAULT_TARGET_TRIPLE STREQUAL TARGET_TRIPLE)
     set(COMPILER_RT_HAS_EXPLICIT_DEFAULT_TARGET_TRIPLE TRUE)
   else()
     set(COMPILER_RT_HAS_EXPLICIT_DEFAULT_TARGET_TRIPLE FALSE)
   endif()
 endmacro()
+
+# Filter out generic versions of routines that are re-implemented in
+# architecture specific manner.  This prevents multiple definitions of the
+# same symbols, making the symbol selection non-deterministic.
+function(filter_builtin_sources output_var exclude_or_include excluded_list)
+  if(exclude_or_include STREQUAL "EXCLUDE")
+    set(filter_action GREATER)
+    set(filter_value -1)
+  elseif(exclude_or_include STREQUAL "INCLUDE")
+    set(filter_action LESS)
+    set(filter_value 0)
+  else()
+    message(FATAL_ERROR "filter_builtin_sources called without EXCLUDE|INCLUDE")
+  endif()
+
+  set(intermediate ${ARGN})
+  foreach (_file ${intermediate})
+    get_filename_component(_name_we ${_file} NAME_WE)
+    list(FIND ${excluded_list} ${_name_we} _found)
+    if(_found ${filter_action} ${filter_value})
+      list(REMOVE_ITEM intermediate ${_file})
+    elseif(${_file} MATCHES ".*/.*\\.S" OR ${_file} MATCHES ".*/.*\\.c")
+      get_filename_component(_name ${_file} NAME)
+      string(REPLACE ".S" ".c" _cname "${_name}")
+      list(REMOVE_ITEM intermediate ${_cname})
+    endif ()
+  endforeach ()
+  set(${output_var} ${intermediate} PARENT_SCOPE)
+endfunction()

compiler-rt/trunk/lib/builtins/CMakeLists.txt

if (NOT MSVC)
   set(x86_64_SOURCES
       x86_64/chkstk.S
       x86_64/chkstk2.S
       x86_64/floatdidf.c
       x86_64/floatdisf.c
       x86_64/floatdixf.c
       x86_64/floatundidf.S
       x86_64/floatundisf.S
-      x86_64/floatundixf.S
-      ${GENERIC_SOURCES})
+      x86_64/floatundixf.S)
+  filter_builtin_sources(x86_64_SOURCES EXCLUDE x86_64_SOURCES "${x86_64_SOURCES};${GENERIC_SOURCES}")
   set(x86_64h_SOURCES ${x86_64_SOURCES})
 
   if (WIN32)
     set(x86_64_SOURCES
         ${x86_64_SOURCES}
         x86_64/chkstk.S
         x86_64/chkstk2.S)
   endif()
set(i386_SOURCES
       i386/floatdixf.S
       i386/floatundidf.S
       i386/floatundisf.S
       i386/floatundixf.S
       i386/lshrdi3.S
       i386/moddi3.S
       i386/muldi3.S
       i386/udivdi3.S
-      i386/umoddi3.S
-      ${GENERIC_SOURCES})
+      i386/umoddi3.S)
+  filter_builtin_sources(i386_SOURCES EXCLUDE i386_SOURCES "${i386_SOURCES};${GENERIC_SOURCES}")
 
   if (WIN32)
     set(i386_SOURCES
         ${i386_SOURCES}
         i386/chkstk.S
         i386/chkstk2.S)
   endif()
 else () # MSVC
set(arm_SOURCES
   arm/sync_fetch_and_umax_4.S
   arm/sync_fetch_and_umax_8.S
   arm/sync_fetch_and_umin_4.S
   arm/sync_fetch_and_umin_8.S
   arm/sync_fetch_and_xor_4.S
   arm/sync_fetch_and_xor_8.S
   arm/udivmodsi4.S
   arm/udivsi3.S
-  arm/umodsi3.S
-  ${GENERIC_SOURCES})
+  arm/umodsi3.S)
+filter_builtin_sources(arm_SOURCES EXCLUDE arm_SOURCES "${arm_SOURCES};${GENERIC_SOURCES}")
 
 set(thumb1_SOURCES
   arm/divsi3.S
   arm/udivsi3.S
   arm/comparesf2.S
   arm/addsf3.S
   ${GENERIC_SOURCES})
 
if (CAN_TARGET_${arch})
       if("${arch}" STREQUAL "armv6m")
         set(_arch "arm|armv6m")
       elseif("${arch}" MATCHES "^(armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
         set(_arch "arm")
       endif()
 
       # Filter out generic versions of routines that are re-implemented in
       # architecture specific manner.  This prevents multiple definitions of the
       # same symbols, making the symbol selection non-deterministic.
       foreach (_file ${${arch}_SOURCES})
         if (${_file} MATCHES ${_arch}/*)
           get_filename_component(_name ${_file} NAME)
           string(REPLACE ".S" ".c" _cname "${_name}")
           list(REMOVE_ITEM ${arch}_SOURCES ${_cname})
         endif ()
       endforeach ()