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

Embed compiler-rt when building programs #7514

Closed
5 tasks
bcardiff opened this issue Mar 6, 2019 · 11 comments
Closed
5 tasks

Embed compiler-rt when building programs #7514

bcardiff opened this issue Mar 6, 2019 · 11 comments

Comments

@bcardiff
Copy link
Member

bcardiff commented Mar 6, 2019

To support overflow and 128 ints property we need to either port the whole library or ship it with the compiler and link against them during compile time (as it's done with the boehm-gc).

I think the plan would be:

  • update omnibus distribution-scripts llvm formula to 6.0.1.
    • This will allow dropping llvm < 4 support in the codebase.
      • Currently the OSX pkg link against llvm 3.9.1.
    • These binaries could also be used in Linux packages if wanted.
      • Currently we use the APK packages for llvm-4.0
  • update omnibus distribution-scripts to build llvm with compiler-rt support.
  • upload llvm + compiler-rt binaries for OSX & Linux in S3
    • as it is done for llvm 3.9.1 for OSX
  • do a release to have the compiler-rt libraries ready to be embedded
  • make compiler link the embedded compiler-rt.

When building compiler-rt there are a bunch of options and they are well described in https://embeddedartistry.com/blog/2017/2/20/compilerrt

  • hard vs soft: this is floating point. Is your platform configured to support hard or soft floating point operations?
  • static vs pic: is the code compiled as a static library, or are you compiling with position-independent-code? (PIC)
  • i386 vs armv7x: this will be dependent upon your platform's processor. You need to pick the instruction set to match.
  • The last portion of the name is the library format.

Some doubts I have are:

  • Should we have a compiler option to choose specifically with which compiler-rt link against?
  • Should we ship all versions of compiler-rt? of all tier-1 platforms?
  • Would be safe to embed compiler-rt 6.0.1 version in Linux that use llvm 4.0? Or should we build compiler-rt 4.0 also?, or update Linux to 6.0.1?
@bcardiff
Copy link
Member Author

bcardiff commented Oct 7, 2019

The current direction is to port compiler-rt to crystal code as done in #8170

Meanwhile, if something is compiler-rt would need to be compiled and linked. More info at https://embeddedartistry.com/blog/2017/2/20/compilerrt in case the environment does not ship compiler-rt directly.

@RX14
Copy link
Contributor

RX14 commented Oct 11, 2019

I'm concerned that a crystal compiler-rt will end up conflicting with other libraries which depend on compiler-rt.

@bcardiff
Copy link
Member Author

@RX14

If the user wants to link an official compiler-rt, the skip_crystal_compiler_rt flag can be used. But I expect the port will be easier to use.
#8170 (comment)

@RX14
Copy link
Contributor

RX14 commented Oct 11, 2019

I suspect we'll have to rejig this later, perhaps mark crystal's symbols as weak symbols, since adding the flag shouldn't be required just because you happened to compile your C library with clang with compiler-rt enabled.

That can be postponed until someone actually hits this in the wild - if you want - though.

@bcardiff
Copy link
Member Author

Yes, supporting weak symbols might be a good idea for this. I didn't thought about that. But at least there is a workaround already.

@RX14
Copy link
Contributor

RX14 commented Oct 21, 2019

Unfortunately, with a bit more googling, weak symbols are not supported by dynamic linkers, on purpose (they used to work but this was disabled for compliance with standards). Which is annoying.

I dug through the LLVM source and found no accessible method of changing the symbol names without a custom LLVM build either.

We must statically link with compiler-rt, properly. The most correct way to do this is to require clang as the linker, but that's not ideal in many other ways. An alternative is to distribute compiler-rt with crystal.

LDC searches for a ldc build of compiler-rt, followed by the one distributed with clang: https://github.com/ldc-developers/ldc/blob/d13ae13308444849559c53a885ad1db127768c4d/driver/linker-gcc.cpp#L267. This is an approach we can copy.

@RX14
Copy link
Contributor

RX14 commented Oct 22, 2019

In fact, I don't think duplicate symbols is a problem at all... ld just chooses one, and the functions are identical. Some actual experimentation is required

@straight-shoota
Copy link
Member

I'm not sure what exactly we're missing from compiler-rt. __muloti4 is obvious for overflowing Int128 multiplication. That can easily be ported from compiler-rt to a Crystal implementation in the same manner as __mulodi4 in #8170. That's a much easier solution than packaging compiler-rt.

Is there anything else?

@Sija
Copy link
Contributor

Sija commented Sep 7, 2021

@straight-shoota Isn't it implemented already as a part of #11111?

@straight-shoota
Copy link
Member

straight-shoota commented Sep 7, 2021

Indeed. Apparently I missed that part. It seemed to be primarily about parsing, not overflow arithmetics. It's also missing specs for this part.

But yeah, it seems like it would resolve this issue.

@straight-shoota
Copy link
Member

I think we can close this. We're now committed to implementing all required functions from compiler-rt in Crystal (latest addition: #11206). This is a bit more effort, but porting C code to Crystal is relatively simple.
This way we can avoid another dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants