Page MenuHomePhabricator

[builtins] Move more float128-related helpers to GENERIC_TF_SOURCES list
ClosedPublic

Authored by atrosinenko on Jun 5 2020, 10:22 AM.

Details

Summary

There are two different _generic_ lists of source files in the compiler-rt/lib/builtins/CMakeLists.txt. Now there is no simple way to not use the tf-variants of helpers at all.

Since there exists a separate GENERIC_TF_SOURCES list, it seems quite natural to move all float128-related helpers there. If it is not possible for some reason, it would be useful to have an explanation of that reason somewhere near the GENERIC_TF_SOURCES definition.

Diff Detail

Event Timeline

atrosinenko created this revision.Jun 5 2020, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2020, 10:22 AM
Herald added subscribers: Restricted Project, mgorny. · View Herald Transcript
  1. Since different quite unrelated patches failed on Jun 5 with quite the same messages, just rebase onto current master expecting build failure to go away
  2. Ping

@samsonov Something strange happens with the compiler-rt tests: when this patch was initially uploaded, the tests were failed with seemingly unrelated failures: B59287. Some tests are broken upstream, I concluded.

Today I have rebased the patch onto current master branch, and the tests passed, as expected: B60775. Right, the tests were actually broken upstream, I concluded.

Then I rebased some other my patch that were failing with similar messages: D81408. There may be some clang-format issues but the thing I not expected were the same test failures I have seen here: B60811. Even more: the B60775 build is marked "Passed", but when I click on "Build 73581 pre-merge checks (buildkite)" and then go to "Exterenl Link: buildkite build" it shows Linux build as failed.

Returning to the original test failures:

linux > cfi-devirt-lld-thinlto-x86_64.cfi-devirt-lld-thinlto-x86_64::anon-namespace.cpp

They contain the following error message:

ld.lld: error: Invalid summary version 8. Version should be in the range [1-7].

This message is seemingly printed by

llvm/lib/Bitcode/Reader/BitcodeReader.cpp
if (Version < 1 || Version > ModuleSummaryIndex::BitcodeSummaryVersion)
  return error("Invalid summary version " + Twine(Version) +
               ". Version should be in the range [1-" +
               Twine(ModuleSummaryIndex::BitcodeSummaryVersion) +
               "].");

with ModuleSummaryIndex::BitcodeSummaryVersion defined to be 8 in current version of repository.

When trying to reproduce this on my local machine, I built LLVM together with clang, compiler-rt and lld. The tests passed. Then I modified the .script file to directly refer to locally built bin/FileCheck, bin/not, etc. and added -v option. Turned out, the Clang driver invokes /path/to/build/directory/bin/ld.lld but after chmod -x bin/lld it silently falls back to /usr/bin/ld.lld. Finally, the build log contains line "-- lld project is disabled" suggesting it does use the system-provided ld.lld that prints this warning.

Look reasonable from my POV, but I don't know enough about CMake or the compiler-rt build system to LGTM this.

efriedma added inline comments.Jun 25 2020, 9:42 AM
compiler-rt/lib/builtins/CMakeLists.txt
141–142

Missed powitf?

Move missed powitf2.c source as well.

atrosinenko marked an inline comment as done.Jun 25 2020, 10:49 AM
atrosinenko added inline comments.
compiler-rt/lib/builtins/CMakeLists.txt
141–142

Fixed, thank you.

This revision is now accepted and ready to land.Jun 25 2020, 12:06 PM
This revision was automatically updated to reflect the committed changes.