Skip to content

Check if the C++ compiler used to build the Concurrency library has the -fswift-async-fp flag #39368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Check if the C++ compiler used to build the Concurrency library has the -fswift-async-fp flag #39368

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions stdlib/public/Concurrency/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,29 @@ endif()
set(SWIFT_RUNTIME_CONCURRENCY_C_FLAGS)
set(SWIFT_RUNTIME_CONCURRENCY_SWIFT_FLAGS)

check_cxx_compiler_flag(-fswift-async-fp=always CXX_COMPILER_HAS_SWIFT_ASYNC_FP_FLAG)

if(NOT swift_concurrency_async_fp_mode)
set(swift_concurrency_async_fp_mode "always")
endif()

# Don't emit extended frame info on platforms other than darwin, system
# backtracer and system debugger are unlikely to support it.
# Don't emit extended frame info on platforms other than darwin, if the compiler
# has that flag, as the system backtracer and system debugger are unlikely to
# support it.
if(CMAKE_SYSTEM_NAME STREQUAL Darwin)
list(APPEND SWIFT_RUNTIME_CONCURRENCY_C_FLAGS
"-fswift-async-fp=${swift_concurrency_async_fp_mode}")
list(APPEND SWIFT_RUNTIME_CONCURRENCY_SWIFT_FLAGS
"-Xfrontend"
"-swift-async-frame-pointer=${swift_concurrency_async_fp_mode}")
else()
if(CXX_COMPILER_HAS_SWIFT_ASYNC_FP_FLAG)
list(APPEND SWIFT_RUNTIME_CONCURRENCY_C_FLAGS
"-fswift-async-fp=${swift_concurrency_async_fp_mode}")
list(APPEND SWIFT_RUNTIME_CONCURRENCY_SWIFT_FLAGS
"-Xfrontend"
"-swift-async-frame-pointer=${swift_concurrency_async_fp_mode}")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to only use this block if the C++ compiler used on macOS supports this flag. That's not working?

Copy link
Contributor

@aschwaighofer aschwaighofer Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not going to work. We normally use the "just-built" compiler (a compiler build from the checked out llvm-project) to build the standard library's c++ files. Unconditionally checking the platform compiler is not the right thing.

Does Android set SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER to use that version of clang? If so you could check if SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER is true -- and only in the case it is true do compiler check logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not going to work. We normally use the "just-built" compiler (a compiler build from the checked out llvm-project) to build the standard library's c++ files. Unconditionally checking the platform compiler is not the right thing.

Oh, right, forgot about the weird way the native build works, as I mostly cross-compile.

Does Android set SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER to use that version of clang?

No, I use the third option, setting --native-clang-tools-path when cross-compiling the standalone stdlib for Android.

I'll look into this more. Sorry about the wild goose chase, @LucianoPAlmeida.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this more. Sorry about the wild goose chase, @LucianoPAlmeida.

No worries, thanks for working on this :)

We normally use the "just-built" compiler (a compiler build from the checked out llvm-project) to build the standard library's c++ files. Unconditionally checking the platform compiler is not the right thing.

Hey @aschwaighofer

If I understand correctly that is true for the ninja builds which are working fine, the original problem we are facing see discussion here is that when building with --xcode the CXX compiler seems to be /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ the one that ships with Xcode version installed which don't have this flag available yet. This happened already sometimes before with other flags for example: https://bugs.swift.org/browse/SR-14714 due the differences of the "just-built" from source clang compiler checked out from apple/llvm-project and the Xcode pre-built clang.
I guess even though they are related to the same flag, the android and Xcode problems are slightly different issues. Does this makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. What we need to happen is to check the compiler that is used to compile the concurrency c++ sources for the availability of the flag.

Which compiler does the check check_cxx_compiler_flag use to perform the check on a standard ninja build? The just build one or the platform toolchain one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would hope whichever one was chosen to build the stdlib, but I'm not sure.

Do you know where the Xcode build chooses to build the stdlib with the platform compiler and not the just-built one? I don't see where that's set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this flag SWIFT_PREBUILT_CLANG that is set based on something passed from somewhere else, but I'm not sure if that is the one doing this ...

else()
message(ERROR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
message(ERROR
message(WARNING

"The -fswift-async-fp clang flag is not available. This is most likely "
"because you are using the Xcode toolchain, which may not have the "
"latest flags yet.")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error should trigger if an older toolchain is used that doesn't have this flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know, we don't even need to error here. Let's say we end up building the C++ parts of the concurrency library with an older Clang. It only affects the back-deployment library, and it only breaks async stack traces through the C++ parts of that library. It's probably more valuable to make this configuration not fail outright than to have it perfect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that you require this flag on macOS, so if the compiler didn't support it, it was better to error here than not pass the flag. You're saying it's okay to build without this flag on macOS when clang doesn't have it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it will break some use cases (stack traces on older macOS versions), but it's not fatal.

endif()
elseif(CXX_COMPILER_HAS_SWIFT_ASYNC_FP_FLAG)
list(APPEND SWIFT_RUNTIME_CONCURRENCY_C_FLAGS "-fswift-async-fp=never")
endif()

Expand Down