[all-commits] [llvm/llvm-project] 8ea148: [Builtins] Fix bug where powerpc builtins speciali...

danliew via All-commits all-commits at lists.llvm.org
Wed Oct 30 16:20:39 PDT 2019


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 8ea148dc0cbff33ac3c80cf4273991465479a01e
      https://github.com/llvm/llvm-project/commit/8ea148dc0cbff33ac3c80cf4273991465479a01e
  Author: Dan Liew <dan at su-root.co.uk>
  Date:   2019-10-30 (Wed, 30 Oct 2019)

  Changed paths:
    M compiler-rt/lib/builtins/CMakeLists.txt

  Log Message:
  -----------
  [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




More information about the All-commits mailing list