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

[WIP/RFC/RFH] Implement compiler-rt libcalls in Julia #18927

Closed
wants to merge 13 commits into from
Closed

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Oct 14, 2016

This is an alternative proposal to #18734 (and still compatible with it)

The idea behind using compiler-rt was for me that it would allow us to use LLVM intrinsics and if those resolve to libcalls (as happens if there is no processor instruction) have a viable fallback.
This is still the case, but there can be an argument made for the case that we would want to be independent of compiler-rt. One of the major drawback of compiler-rt is that it does not support 128bit on 32bit targets, but we currently do.

This uses the current Julia implementations to form the basis of a pure Julia runtime library. (We are still missing a lot of functions).
These are resolved late (and only if required by LLVM). The benefit is that the Float16 implementation becomes more like the way we handle Float32 and Float64.
Also Float128 support is straight forward to implement on all platforms.

I am focusing right now on the conversion functions between floating point numbers and integers, but the list of compiler-rt functions is quite a big longer. (see https://github.com/llvm-mirror/llvm/blob/master/lib/CodeGen/TargetLoweringBase.cpp)

Todo:

  • Decided if we need to prefix our implementations with jl
  • Tests! Tests!
  • PR to BaseBenchmarks with benchmarks for all rtlib functions

@vtjnash Is that the correct usage of jl_extern_c

I would also like to ask for help for the implementation of what we currently are missing.

@vtjnash
Copy link
Member

vtjnash commented Oct 14, 2016

This looks cool. I think this is a correct usage of jl_extern_c, although I think there are some edge cases where it isn't consistent handed right now (specifically, it is a imperative call rather than declarative, so it doesn't deserialize correctly).

Instead of falling back onto compiler-rt we provide our own
implementations. Using compiler-rt is non-trivial and it does not offer
support for 128bit on 32bit platforms. This way we get the benefits of
calling the LLVM intrinsics (possible native intstructions) and we are
can be platform agnostic.
LLVM intrinsics either map to instructions or to functions in
compiler-rt. Since we provide our own implementation we can just look
them up in sys.so and resolve to the function there.

On Darwin we have to use a unmangled version of the function name.
As of compiler-rt 3.9.0 the testsuite uses compareResultH to check for
correctness. That function uses uint16_t under the hood instead of
checking the full 32bit.
@vchuravy
Copy link
Member Author

I have been thinking about the design of RTLIB and I was wondering whether it is a good idea for it to be part of sys.so or if we should have rtlib.so. The hierarchy would be Core -> RTLIB -> Base.

@vtjnash Do you have an idea where the test failure on Mac is coming from? It seems to be a mangling issue.

ccall(:jl_extern_c, Void, (Any, Any, Any, Cstring),
f, rtype, argt, name)

include("rtlib/fp_util.jl")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're moving towards having submodules be in their own folders under base, including their top-level files - so rtlib/RTLIB.jl

Copy link
Member Author

Choose a reason for hiding this comment

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

This might be a bit tricky because I think relative include only becomes available a bit later.

INCLUDE_STATE = 2 # include = _include (from lines above)

I will see if I can move that to an earlier point.

#
# The LLVM Compiler Infrastructure
#
# This file is dual licensed under the MIT and the University of Illinois Open
Copy link
Contributor

Choose a reason for hiding this comment

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

anything copied from LLVM should be acknowledged in the top-level LICENSE.md and added to the exception list in contrib/add_license_to_files.jl

# Note:
# These tests are taken fromt the compiler-rt testsuite. Were as of 3.9.0
# the test are done with compareResultH (so with after casting to UInt16)
# Tests that are commented out fail as === Float32 comparisons.
Copy link
Contributor

Choose a reason for hiding this comment

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

use @test_broken then?

# These tests are taken fromt the compiler-rt testsuite. Were as of 3.9.0
# the test are done with compareResultH (so with after casting to UInt16)
# Tests that are commented out fail as === Float32 comparisons.
# Some tests succedd with ≈ (and are consistent with Julia v0.5 convert)
Copy link
Contributor

Choose a reason for hiding this comment

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

succeed

# The LLVM Compiler Infrastructure
#
# This file is dual licensed under the MIT and the University of Illinois Open
# Source Licenses. See LICENSE.TXT for details.
Copy link
Contributor

@tkelman tkelman Oct 17, 2016

Choose a reason for hiding this comment

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

we don't have a LICENSE.TXT in this repo, so this is a bit confusing

In the earlier phases of building the sysimg the relative form of
`include` is not yet available.
f, rtype, argt, name)

# Check if relative include is available
if isdefined(Base, :INCLUDE_STATE) && Base.INCLUDE_STATE == 1
Copy link
Member Author

Choose a reason for hiding this comment

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

@tkelman Is that an appropriate solution? We can only include files relative to ourselves once Base.INCLUDE_STATE == 2 (and I think this is the only way of testing if we are currently building the sysimg).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, strange. Dunno if the organization is worth this complexity, but I'll let others give opinions.

@staticfloat
Copy link
Member

staticfloat commented Jan 1, 2017

I tried building this just for fun to test out one of my 32-bit Docker images. It broke during bootstrap, with the following message:

rtlib/RTLIB.jl
WARNING: Unable to load sysimage
LLVM ERROR: Program used external function '__gnu_h2f_ieee' which could not be resolved!
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
Makefile:228: recipe for target '/src/julia/usr/lib/julia/sys.o' failed
make[1]: *** [/src/julia/usr/lib/julia/sys.o] Error 1
Makefile:99: recipe for target 'julia-sysimg-release' failed
make: *** [julia-sysimg-release] Error 2

Note that this Docker image passes all tests on a vanilla build of master. Here're the steps I performed to test this: this should be all you need to test locally, assuming you have docker installed:

$ docker run -ti staticfloat/julia_workerbase_ubuntu16_04:x86
root@004e6d44457c# git clone -b vc/jlrt https://github.com/JuliaLang/julia /src/julia
root@004e6d44457c# cd /src/julia; make VERBOSE=1 -j8

@vchuravy
Copy link
Member Author

vchuravy commented Jan 1, 2017 via email

@JeffBezanson
Copy link
Member

This is a cool idea but I'm concerned about re-implementing (and testing and maintaining) a big library that already exists elsewhere. Of course we prefer code to be in julia, but I'm not sure the benefit is worth it for a library like this.

@vchuravy
Copy link
Member Author

vchuravy commented Jul 29, 2017 via email

@tkelman
Copy link
Contributor

tkelman commented Jul 29, 2017

doesn't libquadmath? though we have some alignment and ABI issues to fix before that works properly everywhere IIRC (ref https://github.com/simonbyrne/Quadmath.jl)

@vchuravy
Copy link
Member Author

Ah yes I didn't look at GPL licensed packages to prevent license incumbent.

@vchuravy
Copy link
Member Author

vchuravy commented Mar 9, 2018

superseded by #26381

@vchuravy vchuravy closed this Mar 9, 2018
@DilumAluthge DilumAluthge deleted the vc/jlrt branch March 25, 2021 22:10
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.

5 participants