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

rust-lang/rust integration metabug #66

Closed
8 tasks done
japaric opened this issue Sep 14, 2016 · 8 comments
Closed
8 tasks done

rust-lang/rust integration metabug #66

japaric opened this issue Sep 14, 2016 · 8 comments

Comments

@japaric
Copy link
Member

japaric commented Sep 14, 2016

cc @alexcrichton

@est31
Copy link
Member

est31 commented Jan 5, 2017

Note, now that rust-lang/rust#38482 is merged, i128 support is required as well.

Just using the C implementation is not enough as the C implementation only is enabled and works on non windows 64 bit platforms (that was the reason the PR had to implement the intrinsics in Rust in the first place).

See #133 that attempts to add support.

Also note that during the making of that PR I not just had linker issues with calls to panic, it also complained about calls to eh_personality, which in some cases survives while panic does not.

@japaric
Copy link
Member Author

japaric commented Feb 3, 2017

Added i128 support to the list. After #133 lands, we would still need the famility of floatundif intrinsics, I believe.

Also added changing the ABI of (some?) intrinsics from C to aapcs on ARM. cc #116

compile functions with hidden visibility

And I believe this issue was fixed in rust-lang/rust proper? cc @alexcrichton

@est31
Copy link
Member

est31 commented Feb 4, 2017

With #133 merged, the following float functions are required to be ported:

  • __fixsfti
  • __fixdfti
  • __fixunsdfti
  • __fixunssfti
  • __floattidf
  • __floattisf
  • __floatuntidf
  • __floatuntisf

They all need extern "unadjusted" instead of extern "C" on windows.

bors added a commit that referenced this issue Feb 7, 2017
use AAPCS calling convention on all aeabi intrinsics

also, on ARM, inline(always) the actual implementation of the intrinsics so we
end with code like this:

```
00000000 <__aeabi_dadd>:
    (implementation here)
```

instead of "trampolines" like this:

```
00000000 <__aeabi_dadd>:
    (shuffle registers)
    (call __adddf3)

00000000 <__adddf3>:
    (implementation here)
```

closes #116

cc #66
r? @alexcrichton
cc @mattico
bors added a commit that referenced this issue Feb 8, 2017
use AAPCS calling convention on all aeabi intrinsics

also, on ARM, inline(always) the actual implementation of the intrinsics so we
end with code like this:

```
00000000 <__aeabi_dadd>:
    (implementation here)
```

instead of "trampolines" like this:

```
00000000 <__aeabi_dadd>:
    (shuffle registers)
    (call __adddf3)

00000000 <__adddf3>:
    (implementation here)
```

closes #116

cc #66
r? @alexcrichton
cc @mattico
@japaric
Copy link
Member Author

japaric commented Feb 8, 2017

"C" ABI -> "aapcs" on ARM - #116 #141

I think that powisf2 and powidf2 may need to be changed to AAPCS too. Honestly, I'm no longer sure what's the state of those intrinsics on LLVM 3.9 vs 4.0.

@japaric
Copy link
Member Author

japaric commented Jun 3, 2017

OK. I believe that all the required intrinsics to land this in rust-lang/rust have been implemented. All that's left is: (a) check that the compiler-rt submodule in this repo matches the revision being used in rust-lang/rust and then (b) fight with bors and land this in rust-lang/rust.

Any volunteers? Here's what needs to be done for (b):

  • Remove the src/libcompiler_builtins directory in rust-lang/rust
  • Replace it with a git submodule that points to this repo
  • Create a libcompiler_builtins_shim Cargo project that builds this repo with the right Cargo features (the "rustbuild" feature). The std crate will depend on that shim. (I lost track of the comment that explained why the shim was necessary. Sorry)
  • Likely update the part of the boostrap tool that cares of building the sanitizers' runtimes to use the new location of the compiler-rt code. (src/compiler-rt -> src/libcompiler_builtins/compiler-rt)

All in one PR. You should test locally as a sanity check and then we can feed it to bors.

Here's an old attempt of mine: rust-lang/rust#37802. All the Makefile changes in that PR won't be necessary as the Makefiles have been removed.

@est31
Copy link
Member

est31 commented Jun 3, 2017

Not a volunteer, but as someone who had a good share of "fighting with bors", I wish anyone who mounts an attempt the best of luck.

@japaric
Copy link
Member Author

japaric commented Jun 3, 2017

@est31 Hey, don't scare away the volunteers!

@alexcrichton
Copy link
Member

Done now!

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

No branches or pull requests

3 participants