Skip to content
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.

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

Shim llvm::sys::getDefaultTargetTriple() to Swift #572

Closed
wants to merge 1 commit into from

Conversation

beccadax
Copy link

@beccadax beccadax commented Oct 3, 2019

The default target triple for the host machine can be useful information when constructing command line arguments for a tool, but this information isn’t in LLVM’s C API, so it’s inaccessible from Swift. This commit exposes it in libllbuild as llb_get_default_target_triple() and in llbuildSwift as BuildSystem.defaultTargetTriple.

The default target triple for the host machine can be useful information when constructing command line arguments for a tool, but this information isn’t in LLVM’s C API, so it’s inaccessible from Swift. This commit exposes it in libllbuild as `llb_get_default_target_triple()` and in llbuildSwift as `BuildSystem.defaultTargetTriple`.
@beccadax
Copy link
Author

beccadax commented Oct 3, 2019

@swift-ci please smoke test


const char * llb_get_default_target_triple() {
// Use a static local to store the triple, to avoid lifetime issues.
static std::string tripleString = llvm::sys::getDefaultTargetTriple();
Copy link
Member

Choose a reason for hiding this comment

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

does this really work? LLVM goes to some extreme lengths to determine the default triple. I am not sure if llbuild does that as well.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not quite sure what you mean. This ends up using the exact same code that would be called in the clang or Swift drivers. That code does sometimes invoke external tools or perform other heroics to determine the OS version, but all of that code looks like it's intended to be idempotent, so caching the result ought not to cause any problems.

Copy link
Member

Choose a reason for hiding this comment

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

I meant if the copy in llbuild actually does the heroics done by llvm. I checked and it seems like this is filled in at compile time using LLVM_DEFAULT_TARGET_TRIPLE defined to empty string in llbuild: https://github.com/apple/swift-llbuild/blob/master/include/llvm/Config/config.h#L356

Copy link
Author

Choose a reason for hiding this comment

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

Damn, you're right. Is there a particular reason we do this, or is it just because we haven't wanted this until now?

@beccadax
Copy link
Author

beccadax commented Oct 3, 2019

Looks like this won't work without a larger change to the way we configure llbuild.

@beccadax beccadax closed this Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants