[builtins] Cleanup generic-file filtering

Split filter_builtin_sources into two functions:
 - filter_builtin_sources that removes generic files when an
   arch-specific file is selected.
 - darwin_filter_builtin_sources that implements the EXCLUDE/INCLUDE
   lists (using the files in lib/builtins/Darwin-excludes).

darwin_filter_builtin_sources delegates to filter_builtin_sources.

Previously, lib/builtins/CMakeLists.txt had a number of calls to
filter_builtin_sources (with a confusing/broken use of the
`excluded_list` parameter), as well as a redundant arch-vs-generic
filtering for the non-Apple code path at the end of the file. Replace
all of this with a single call to filter_builtin_sources.

Remove i686_SOURCES. Previously, this list contained only the
arch-specific files common to 32-bit and 64-bit x86, which is a strange
set. Normally the ${ARCH}_SOURCES list contains everything needed for
the arch. "i686" isn't in ALL_BUILTIN_SUPPORTED_ARCH.

NFCI, but i686_SOURCES won't be defined, and the order of files in
${arch}_SOURCES lists will change.

Differential Revision: https://reviews.llvm.org/D82151

compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake

@@ -344,6 +344,38 @@ function(darwin_lipo_libs name)
   endif()
 endfunction()
 
+# Filter the list of builtin sources for Darwin, then delegate to the generic
+# filtering.
+#
+# `exclude_or_include` must be one of:
+#  - EXCLUDE: remove every item whose name (w/o extension) matches a name in
+#    `excluded_list`.
+#  - INCLUDE: keep only items whose name (w/o extension) matches something
+#    in `excluded_list`.
+function(darwin_filter_builtin_sources output_var name 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})
+    endif()
+  endforeach()
+
+  filter_builtin_sources(intermediate ${name})
+  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)
@@ -366,7 +398,8 @@ macro(darwin_add_builtin_libraries)
                               ARCH ${arch}
                               MIN_VERSION ${DARWIN_${os}_BUILTIN_MIN_VER})
 
-      filter_builtin_sources(filtered_sources
+      darwin_filter_builtin_sources(filtered_sources
+        ${os}_${arch}
         EXCLUDE ${arch}_${os}_EXCLUDED_BUILTINS
         ${${arch}_SOURCES})
 
@@ -388,7 +421,8 @@ macro(darwin_add_builtin_libraries)
                               OS ${os}
                               ARCH ${arch})
 
-        filter_builtin_sources(filtered_sources
+        darwin_filter_builtin_sources(filtered_sources
+          cc_kext_${os}_${arch}
           EXCLUDE ${arch}_${os}_EXCLUDED_BUILTINS
           ${${arch}_SOURCES})
 
@@ -484,7 +518,8 @@ macro(darwin_add_embedded_builtin_libraries)
     set(x86_64_FUNCTIONS ${common_FUNCTIONS})
 
     foreach(arch ${DARWIN_macho_embedded_ARCHS})
-      filter_builtin_sources(${arch}_filtered_sources
+      darwin_filter_builtin_sources(${arch}_filtered_sources
+        macho_embedded_${arch}
         INCLUDE ${arch}_FUNCTIONS
         ${${arch}_SOURCES})
       if(NOT ${arch}_filtered_sources)

compiler-rt/cmake/Modules/CompilerRTUtils.cmake

@@ -332,33 +332,30 @@ macro(construct_compiler_rt_default_triple)
   endif()
 endmacro()
 
-# Filter out generic versions of routines that are re-implemented in
+# Filter out generic versions of routines that are re-implemented in an
-# architecture specific manner.  This prevents multiple definitions of the
+# architecture specific manner. This prevents multiple definitions of the same
-# same symbols, making the symbol selection non-deterministic.
+# symbols, making the symbol selection non-deterministic.
-function(filter_builtin_sources output_var exclude_or_include excluded_list)
+#
-  if(exclude_or_include STREQUAL "EXCLUDE")
+# We follow the convention that a source file that exists in a sub-directory
-    set(filter_action GREATER)
+# (e.g. `ppc/divtc3.c`) is architecture-specific and that if a generic
-    set(filter_value -1)
+# implementation exists it will be a top-level source file with the same name
-  elseif(exclude_or_include STREQUAL "INCLUDE")
+# modulo the file extension (e.g. `divtc3.c`).
-    set(filter_action LESS)
+function(filter_builtin_sources inout_var name)
-    set(filter_value 0)
+  set(intermediate ${${inout_var}})
-  else()
+  foreach(_file ${intermediate})
-    message(FATAL_ERROR "filter_builtin_sources called without EXCLUDE|INCLUDE")
+    get_filename_component(_file_dir ${_file} DIRECTORY)
-  endif()
+    if (NOT "${_file_dir}" STREQUAL "")
-
+      # Architecture specific file. If a generic version exists, print a notice
-  set(intermediate ${ARGN})
+      # and ensure that it is removed from the file list.
-  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}")
+      string(REGEX REPLACE "\\.S$" ".c" _cname "${_name}")
-      list(REMOVE_ITEM intermediate ${_cname})
+      if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${_cname}")
-    endif ()
+        message(STATUS "For ${name} builtins preferring ${_file} to ${_cname}")
-  endforeach ()
+        list(REMOVE_ITEM intermediate ${_cname})
-  set(${output_var} ${intermediate} PARENT_SCOPE)
+      endif()
+    endif()
+  endforeach()
+  set(${inout_var} ${intermediate} PARENT_SCOPE)
 endfunction()
 
 function(get_compiler_rt_target arch variable)

compiler-rt/lib/builtins/CMakeLists.txt

@@ -260,7 +260,9 @@ endif ()
 
 if (NOT MSVC)
   set(x86_64_SOURCES
+    ${GENERIC_SOURCES}
     ${GENERIC_TF_SOURCES}
+    ${x86_ARCH_SOURCES}
     x86_64/floatdidf.c
     x86_64/floatdisf.c
     x86_64/floatdixf.c
@@ -268,7 +270,8 @@ if (NOT MSVC)
     x86_64/floatundisf.S
     x86_64/floatundixf.S
   )
-  filter_builtin_sources(x86_64_SOURCES EXCLUDE x86_64_SOURCES "${x86_64_SOURCES};${GENERIC_SOURCES}")
+
+  # Darwin x86_64 Haswell
   set(x86_64h_SOURCES ${x86_64_SOURCES})
 
   if (WIN32)
@@ -280,6 +283,8 @@ if (NOT MSVC)
   endif()
 
   set(i386_SOURCES
+    ${GENERIC_SOURCES}
+    ${x86_ARCH_SOURCES}
     i386/ashldi3.S
     i386/ashrdi3.S
     i386/divdi3.S
@@ -295,7 +300,6 @@ if (NOT MSVC)
     i386/udivdi3.S
     i386/umoddi3.S
   )
-  filter_builtin_sources(i386_SOURCES EXCLUDE i386_SOURCES "${i386_SOURCES};${GENERIC_SOURCES}")
 
   if (WIN32)
     set(i386_SOURCES
@@ -309,20 +313,15 @@ else () # MSVC
   # MSVC's assembler takes Intel syntax, not AT&T syntax.
   # Also use only MSVC compilable builtin implementations.
   set(x86_64_SOURCES
+    ${GENERIC_SOURCES}
+    ${x86_ARCH_SOURCES}
     x86_64/floatdidf.c
     x86_64/floatdisf.c
     x86_64/floatdixf.c
-    ${GENERIC_SOURCES}
   )
-  set(x86_64h_SOURCES ${x86_64_SOURCES})
+  set(i386_SOURCES ${GENERIC_SOURCES} ${x86_ARCH_SOURCES})
-  set(i386_SOURCES ${GENERIC_SOURCES})
 endif () # if (NOT MSVC)
 
-set(x86_64h_SOURCES ${x86_64h_SOURCES} ${x86_ARCH_SOURCES})
-set(x86_64_SOURCES ${x86_64_SOURCES} ${x86_ARCH_SOURCES})
-set(i386_SOURCES ${i386_SOURCES} ${x86_ARCH_SOURCES})
-set(i686_SOURCES ${i686_SOURCES} ${x86_ARCH_SOURCES})
-
 set(arm_SOURCES
   arm/fp_mode.c
   arm/bswapdi2.S
@@ -356,8 +355,8 @@ set(arm_SOURCES
   arm/udivmodsi4.S
   arm/udivsi3.S
   arm/umodsi3.S
+  ${GENERIC_SOURCES}
 )
-filter_builtin_sources(arm_SOURCES EXCLUDE arm_SOURCES "${arm_SOURCES};${GENERIC_SOURCES}")
 
 set(thumb1_SOURCES
   arm/divsi3.S
@@ -451,8 +450,8 @@ if(MINGW)
     arm/aeabi_uldivmod.S
     arm/chkstk.S
     mingw_fixfloat.c
+    ${GENERIC_SOURCES}
   )
-  filter_builtin_sources(arm_SOURCES EXCLUDE arm_SOURCES "${arm_SOURCES};${GENERIC_SOURCES}")
 elseif(NOT WIN32)
   # TODO the EABI sources should only be added to EABI targets
   set(arm_SOURCES
@@ -619,25 +618,8 @@ else ()
         endif()
       endif()
 
-      # Filter out generic versions of routines that are re-implemented in
+      # Remove a generic C builtin when an arch-specific builtin is specified.
-      # architecture specific manner.  This prevents multiple definitions of the
+      filter_builtin_sources(${arch}_SOURCES ${arch})
-      # same symbols, making the symbol selection non-deterministic.
-      foreach (_file ${${arch}_SOURCES})
-        get_filename_component(_file_dir "${_file}" DIRECTORY)
-        if (NOT "${_file_dir}" STREQUAL "")
-          # Architecture specific file. We follow the convention that a source
-          # file that exists in a sub-directory (e.g. `ppc/divtc3.c`) is
-          # architecture specific and that if a generic implementation exists
-          # it will be a top-level source file with the same name modulo the
-          # file extension (e.g. `divtc3.c`).
-          get_filename_component(_name ${_file} NAME)
-          string(REPLACE ".S" ".c" _cname "${_name}")
-          if (EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${_cname}")
-            message(STATUS "For ${arch} builtins preferring ${_file} to ${_cname}")
-            list(REMOVE_ITEM ${arch}_SOURCES ${_cname})
-          endif()
-        endif ()
-      endforeach ()
 
       # Needed for clear_cache on debug mode, due to r7's usage in inline asm.
       # Release mode already sets it via -O2/3, Debug mode doesn't.