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

Replace {u,i}128_* lang items with __rust_{u,i}128_* unmangled functions #302

Merged
merged 2 commits into from
Jul 19, 2019
Merged

Replace {u,i}128_* lang items with __rust_{u,i}128_* unmangled functions #302

merged 2 commits into from
Jul 19, 2019

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jul 13, 2019

The -Zlower-128bit-ops feature is completely broken, as libcore needs
those lang items to compile with this feature, but they are only
provided by compiler_builtins, which itself depends on libcore.
According to rust-lang/rust#58969 the feature never got finished.

This commit removes the associated lang items and replaces them with
normal unmangled functions, when there is no existing intrinsic. This
makes it easier for alternative codegen backends to implement 128bit
integer support.

cc https://github.com/bjorn3/rustc_codegen_cranelift/pull/627

The -Zlower-128bit-ops feature is completely broken, as libcore needs
those lang items to compile with this feature, but they are only
provided by compiler_builtins, which itself depends on libcore.
According to rust-lang/rust#58969 the feature never got finished.

This commit removes the associated lang items and replaces them with
normal unmangled functions, when there is no existing intrinsic. This
makes it easier for alternative codegen backends to implement 128bit
integer support.
@alexcrichton
Copy link
Member

Thanks!

Do you know if this works with presumably accompanied changes to the trans backend for rustc? If not I might recommend not landing this here and just using [patch] locally to develop against this and make sure it works in conjunction with updates to rustc.

@bjorn3
Copy link
Member Author

bjorn3 commented Jul 15, 2019

I plan to remove -Zlower-128bit-ops from rustc after this is merged. The llvm codegen backend will likely not use those new __rust_* functions, but I want to use them in bjorn3/rustc_codegen_cranelift#627. Removing the lang item definitions should not break anything, because they are only used when -Zlower-128bit-ops is passed, which was broken anyway.

@alexcrichton
Copy link
Member

Ok makes sense! I'm basically asking that things be working before we merge this. I'd prefer to not have to merge this and then publish a number of subsequent bugfix releases. I'm also not sure if there's much benefit to merging ahead of time before it's tested to work, but I could be wrong!

@bjorn3
Copy link
Member Author

bjorn3 commented Jul 15, 2019

I'd prefer to not have to merge this and then publish a number of subsequent bugfix releases.

I understand. I will probably make the rustc changes later this week and test it.

I'm also not sure if there's much benefit to merging ahead of time before it's tested to work, but I could be wrong!

Not much. bjorn3/rustc_codegen_cranelift#627 is blocked on bytecodealliance/cranelift#795 anyway.

@alexcrichton
Copy link
Member

Ok, thanks! Let me know when it's all ship shape and I can merge/publish!

@bjorn3
Copy link
Member Author

bjorn3 commented Jul 15, 2019

Sure

@bjorn3
Copy link
Member Author

bjorn3 commented Jul 19, 2019

I tested rust-lang/rust#62801 together with this and it seemed to work on x86_64. (didnt have time to run all tests though, but libstd and rustc built)

@alexcrichton
Copy link
Member

Ok thanks!

@alexcrichton alexcrichton merged commit e578d47 into rust-lang:master Jul 19, 2019
@alexcrichton
Copy link
Member

I've published this as 0.1.18

@bjorn3 bjorn3 deleted the no_128bit_lang_items branch July 19, 2019 14:46
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this pull request Jul 25, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
…lexcrichton

Remove support for -Zlower-128bit-ops

It is broken and unused

cc rust-lang#58969

blocked rust-lang/compiler-builtins#302 (removes definitions of the lang items removed in this PR)

r? @alexcrichton
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.

2 participants