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

Add missing target-abi to cross-lang llvm lto bitcode #123400

Closed
wants to merge 1 commit into from

Conversation

yujincheng08
Copy link
Contributor

When linker_plugin_lto enabled, we need to add target-abi to the generated llvm bitcode, otherwise there may be an error like "cannot link object files with different floating-point ABI".

When `linker_plugin_lto` enabled, we need to add `target-abi` to
the generated llvm bitcode, otherwise there may be an error like
"cannot link object files with different floating-point ABI".
@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 3, 2024
@yujincheng08 yujincheng08 changed the title Add missing target-abi to corss-lang llvm lto bitcode Add missing target-abi to cross-lang llvm lto bitcode Apr 3, 2024
@compiler-errors
Copy link
Member

r? @wesleywiser or @cuviper perhaps?

@yujincheng08
Copy link
Contributor Author

r? @cuviper

@rustbot rustbot assigned cuviper and unassigned wesleywiser Apr 6, 2024
@cuviper
Copy link
Member

cuviper commented Apr 8, 2024

Is there a test we can add for this?

For most targets, this will be an empty string, and we already set it when creating the target machine. I'm surprised that this would need to be repeated in module options, and I can only find one place where clang does this, only on RISC-V:
https://github.com/llvm/llvm-project/blob/d345f6a25343f926f55e70b442dfa507ba31b597/clang/lib/CodeGen/CodeGenModule.cpp#L1089-L1093

@rust-lang/wg-llvm, anyone else have a better idea of expectations here?

@DianQK
Copy link
Member

DianQK commented Apr 9, 2024

I'd like to first see a specific error log if adding a test case is challenging.

@nikic
Copy link
Contributor

nikic commented Apr 9, 2024

It looks like another PR for the same issue with a test has been opened here: #123612

@yujincheng08
Copy link
Contributor Author

Thanks. close in flavor of #123612

@yujincheng08 yujincheng08 deleted the targetabi branch April 9, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants