Remove dependency on libatomic on Linux #29581
Conversation
Due to some unfortunate interplay between clang and libstdc++, clang was not able to correctly identify to alignment of PoolRange and SideTableRefCountBits, causing it to emit library calls instead of inlining atomic operations. This was fixed by adding the appropriate alignment to those types. In addition to that the march for the Linux target was set to 'core2', which is the earliest architecture to support cx16, which is necessary for the atomic operations on PoolRange.
|
preset=buildbot_incremental_linux,tsan-libdispatch-test |
1 similar comment
|
preset=buildbot_incremental_linux,tsan-libdispatch-test |
|
Wow! Nice catch on that. This is actually going to simplify a few things. |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Build failed |
|
I don't think that |
|
Yeah, I don't know why it's failing in the first place. It emits __sync call in the LongRefcountingTest. That's why I tried adding the march there as well. I have so far been unable to reproduce this locally. |
|
@swift-ci please test Linux platform |
|
Build failed |
|
Ok, just reproduced the __sync issue in a 16.04 VM. Working on a fix. |
|
@swift-ci please test Linux platform |
|
Build failed |
|
@swift-ci please smoke test |
|
@swift-ci please test |
|
Build failed |
|
@compnerd Figured it out. The problem was that I used |
| set_property(TARGET "${test_dirname}" APPEND PROPERTY LINK_LIBRARIES | ||
| "atomic") | ||
| if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64") | ||
| set_property(TARGET "${test_dirname}" APPEND_STRING PROPERTY COMPILE_FLAGS |
There was a problem hiding this comment.
I think that this is better written as:
target_compile_options(${test_dirname} PRIVATE
-march=core2)There was a problem hiding this comment.
No strong opinion, but it's done with set_property in the rest of the file, so maybe it's worth keeping it consistent?
There was a problem hiding this comment.
The target_* form is more concise, avoids string manipulations, and should reduce memory overheads for CMake having to translate between representations. It also will make it easier to split apart the host and target builds.
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
… and Haiku Follow suit from linux, swiftlang#29581, tested on Android.
…and Haiku Follow suit from linux, swiftlang#29581, tested on Android.
Due to some unfortunate interplay between clang and libstdc++, clang was
not able to correctly identify to alignment of PoolRange and
SideTableRefCountBits, causing it to emit library calls instead of
inlining atomic operations. This was fixed by adding the appropriate
alignment to those types. In addition to that the march for the Linux
target was set to 'core2', which is the earliest architecture to support
cx16, which is necessary for the atomic operations on PoolRange.