forked from OSchip/llvm-project
[Builtins] Fix bug where powerpc builtins specializations didn't remove generic implementations.
Summary: Previously the CMake code looked for filepaths of the form `<arch>/<filename>` as an indication that `<arch>/<filename>` provided a specialization of a top-level file `<filename>`. For powerpc there was a bug because the powerpc specialized implementations lived in `ppc/` but the architectures were `powerpc64` and `powerpc64le` which meant that CMake was looking for files at `powerpc64/<filename>` and `powerpc64le/<filename>`. The result of this is that for powerpc the builtins library contained a duplicate symbol for `divtc3` because it had the generic implementation and the specialized version in the built static library. Although we could just add similar code to what there is for arm (i.e. compute `${_arch}`) to fix this, this is extremely error prone (until r375150 no error was raised). Instead this patch takes a different approach that removes looking for the architecture name entirely. Instead this patch uses the convention that a source file in a sub-directory might be a specialization of a generic implementation and if a source file of the same name (ignoring extension) exists at the top-level then it is the corresponding generic implementation. This approach is much simpler because it doesn't require keeping track of different architecture names. This convention already existed in repository but previously it was implicit. This change makes it explicit. This patch is motivated by wanting to revert r375162 which worked around the powerpc bug found when r375150 landed. Once it lands we should revert r375162. Reviewers: phosek, beanz, compnerd, shiva0217, amyk, rupprecht, kongyi, mstorsjo, t.p.northover, weimingz, jroelofs, joerg, sidneym Subscribers: nemanjai, mgorny, kristof.beyls, jsji, shchenz, steven.zhang, #sanitizers, llvm-commits Tags: #llvm, #sanitizers Differential Revision: https://reviews.llvm.org/D69189
This commit is contained in:
parent
c3b06d0c39
commit
8ea148dc0c
|
@ -586,15 +586,6 @@ else ()
|
||||||
|
|
||||||
foreach (arch ${BUILTIN_SUPPORTED_ARCH})
|
foreach (arch ${BUILTIN_SUPPORTED_ARCH})
|
||||||
if (CAN_TARGET_${arch})
|
if (CAN_TARGET_${arch})
|
||||||
# NOTE: some architectures (e.g. i386) have multiple names. Ensure that
|
|
||||||
# we catch them all.
|
|
||||||
set(_arch ${arch})
|
|
||||||
if("${arch}" STREQUAL "armv6m")
|
|
||||||
set(_arch "arm|armv6m")
|
|
||||||
elseif("${arch}" MATCHES "^(armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
|
|
||||||
set(_arch "arm")
|
|
||||||
endif()
|
|
||||||
|
|
||||||
# For ARM archs, exclude any VFP builtins if VFP is not supported
|
# For ARM archs, exclude any VFP builtins if VFP is not supported
|
||||||
if (${arch} MATCHES "^(arm|armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
|
if (${arch} MATCHES "^(arm|armhf|armv7|armv7s|armv7k|armv7m|armv7em)$")
|
||||||
string(REPLACE ";" " " _TARGET_${arch}_CFLAGS "${TARGET_${arch}_CFLAGS}")
|
string(REPLACE ";" " " _TARGET_${arch}_CFLAGS "${TARGET_${arch}_CFLAGS}")
|
||||||
|
@ -608,10 +599,19 @@ else ()
|
||||||
# architecture specific manner. This prevents multiple definitions of the
|
# architecture specific manner. This prevents multiple definitions of the
|
||||||
# same symbols, making the symbol selection non-deterministic.
|
# same symbols, making the symbol selection non-deterministic.
|
||||||
foreach (_file ${${arch}_SOURCES})
|
foreach (_file ${${arch}_SOURCES})
|
||||||
if (${_file} MATCHES ${_arch}/*)
|
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)
|
get_filename_component(_name ${_file} NAME)
|
||||||
string(REPLACE ".S" ".c" _cname "${_name}")
|
string(REPLACE ".S" ".c" _cname "${_name}")
|
||||||
list(REMOVE_ITEM ${arch}_SOURCES ${_cname})
|
if (EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${_cname}")
|
||||||
|
message(STATUS "For ${arch} builtins preferring ${_file} to ${_cname}")
|
||||||
|
list(REMOVE_ITEM ${arch}_SOURCES ${_cname})
|
||||||
|
endif()
|
||||||
endif ()
|
endif ()
|
||||||
endforeach ()
|
endforeach ()
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue