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

compiler-rt #12027

Merged
merged 2 commits into from
Feb 12, 2014
Merged

compiler-rt #12027

merged 2 commits into from
Feb 12, 2014

Conversation

vadimcn
Copy link
Contributor

@vadimcn vadimcn commented Feb 4, 2014

This is an attempt to remove some more of Rust's dependencies on libgcc and replace it with LLVM's compiler-rt lib. I've added compiler-rt as a submodule and changed libstd to link with it.
As far as I could verify, after this change, the only symbols still imported by std from libgcc are the stack unwinding functions. Other crates, however, still picked up symbols from libgcc, not from libstd, as I had hoped. So linking definitely requires some work.

I've only tested this on windows, 32-bit linux and android and fully expect it to fail on other platforms. Patches are welcome.

@omasanori
Copy link
Contributor

Why should we bundle one more library into Rust even on platforms that already have libgcc or compiler-rt as built-in? How about adding a ./configure flag for those platforms?

@alexcrichton
Copy link
Member

We prefer having a knowledge of all dependencies that we're bringing in, and we prefer even more about having control over building them (as long as we can fairly easily build them). The compiler-rt library's presence is expected by LLVM because codegen on various platforms will lower LLVM instructions to function calls into the compiler-rt library. An example of this is #8449.

By bundling compiler-rt, we can both have a better understanding of what our dependencies are, plus we are going to be able to correctly lower all valid LLVM IR to something that we can compile.

@alexcrichton
Copy link
Member

This is an amazing effort, thank you for doing this! Some thoughts:

  • I would expect all rust programs to depend on compiler-rt, similarly to how all rust programs depend on libmorestack. You should be able to compile various parts of rust without the standard library.
  • I think we'll want to abuse the llvm-auto-clean-trigger for also triggering a rebuild of compiler-rt (I can help with the makefiles)
  • I'm a little worried about the number of patches which you have on top of compiler-rt master, but we haven't made a fantastic effort yet to upstream our patches, so this is probably fine for now. We'll also want to put the fork at rust-lang/compiler-rt (like llvm/libuv/gyp/etc).
  • To prevent the inclusion of libgcc, we may want to start using -nodefaultlibs on platforms.

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 4, 2014

So you mean we should just throw -lcompiler-rt into the linker command line? Do we really want each shared module to have its' own copy of all that stuff?

I'll try to upstream my changes to compiler-rt as soon as I figure out the procedure for doing this.

Can't use -nodefaultlibs just yet, because we still need stack unwinding functions from libgcc. Well, we could try using the system unwind library, where available, or libgcc_eh. But I still don't know what to do about Windows, as stack unwind info registration depends on having libgcc_s_dw2.dll in the process.

@thestinger
Copy link
Contributor

@vadimcn: I think each shared object is intended to have a copy of it. They're tiny support functions and I doubt they're used much on x86.

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 4, 2014

@thestinger: maybe, but currently this is not the case - they are all imported from shared libgcc (unless one forces static linking).

@alexcrichton
Copy link
Member

@vadimcn, I added some -nodefaultlibs support in #12031, but I couldn't figure out windows sadly (couldn't figure out how to compile a printf with -nodefaultlibs)

I, too, thought this was just a static library which provided small shims that LLVM lowers to. Is it not recommended to link complier-rt statically into programs? The linker should discard any unused objects, so I wouldn't be worried about size at all.

@thestinger
Copy link
Contributor

@vadimcn: The compiler-rt library has one tiny support function per object file so any unused functions are not included. It would be interesting to see how many of these functions are actually used on x86_64. I don't think there are many. They're only called by the target's code generation when it can't do anything better.

@brson
Copy link
Contributor

brson commented Feb 6, 2014

Is it feasible to make compiler-rt a crate?

@thestinger
Copy link
Contributor

@brson: It could probably be wrapped as a crate, but no one should ever be using it directly. It exists solely for LLVM CodeGen to use as a last resort when there's no platform capability to do the operation. For example, integer division on platforms without CPU integer division. I don't think it has any sort of backwards compatibility guarantee.

@alexcrichton
Copy link
Member

@vadimcn, would you like me to take over from here? Also, do you have an update on upstreaming the compiler-rt patches?

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 11, 2014

I've submitted my patches to llvm-commits, but it hadn't made quite the splash I had hoped for [1]. I was about to ping them again.

[1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-February/thread.html#70117

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 11, 2014

@alexcrichton: added your makefile changes

@@ -1130,6 +1131,18 @@ fn link_args(sess: Session,
args.push(~"-shared-libgcc");
}

if sess.targ_cfg.os == abi::OsAndroid {
// Enable multiple symbol definitions,- to be consistent with other platforms.
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment here explaining about why this is needed in conjuction with comiler-rt?

@alexcrichton
Copy link
Member

Could you also add alexcrichton@b37700c to this PR to see whether it can land with it?

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 11, 2014

I can't test your -nodefaultlibs PR because I don't have a mac. I expect that my PR will cause breakage on its' own on platforms I didn't test, so I am thinking they will be easier to land separately.

@alexcrichton
Copy link
Member

OK, I'll try to land it later.

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 12, 2014

updated

bors added a commit that referenced this pull request Feb 12, 2014
This is an attempt to remove some more of Rust's dependencies on libgcc and replace it with LLVM's compiler-rt lib.  I've added compiler-rt as a submodule and changed libstd to link with it.  
As far as I could verify, after this change, the only symbols still imported by std from libgcc are the stack unwinding functions.   Other crates, however, still picked up symbols from libgcc, not from libstd, as I had hoped.  So linking definitely requires some work. 

I've only tested this on windows, 32-bit linux and android and fully expect it to fail on other platforms. Patches are welcome.
@bors bors closed this Feb 12, 2014
@bors bors merged commit b765132 into rust-lang:master Feb 12, 2014
@alexcrichton
Copy link
Member

Wow, that was surprisingly painless, nice job!

@brson
Copy link
Contributor

brson commented Feb 12, 2014

Congrats.

@vadimcn
Copy link
Contributor Author

vadimcn commented Feb 12, 2014

Whoa, I'll be damned!

@thestinger thestinger mentioned this pull request Feb 14, 2014
4 tasks
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 15, 2014
This was just waiting for compiler-rt support, which was added in rust-lang#12027

Closes rust-lang#8449
bors added a commit that referenced this pull request Feb 15, 2014
This was just waiting for compiler-rt support, which was added in #12027

Closes #8449
@vadimcn vadimcn deleted the compiler-rt branch August 8, 2014 21:43
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
…tate, r=Veykril

fix: Try not to invalidate state when the proc macro preference didn't change

This appears to fix rust-lang#12027, but I'm not sure.
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.

6 participants