morganstanley / hobbes

A language and an embedded JIT compiler
http://hobbes.readthedocs.io/
Apache License 2.0
1.16k stars 105 forks source link

Support for LLVM 9 #315

Closed Chaz6 closed 3 years ago

Chaz6 commented 4 years ago

LLVM 9 is the current release of LLVM. Building currently fails with it.

kthielen commented 4 years ago

Maybe redundant with: https://github.com/Morgan-Stanley/hobbes/issues/294

There's an alternative compiler backend in hobbes now (the header-only lib in include/hobbes/mc/). It might make sense to move to this instead, the moving target of LLVM JIT support has caused a lot of pain.

snow-abstraction commented 4 years ago

A first step would be to 1.) document which versions are supported 2.) make cmake fail if no supported versions 3.) make sure the preprocesser stuff (i.e. #if LLVM_VERSION_MAJOR == 6 ... ) is in sync with the above.

snow-abstraction commented 4 years ago

It seems that cmake does not support having a list of supported versions so perhaps only my 1.) suggestion is actually applicable.

kthielen commented 4 years ago

Unfortunately, LLVM's JIT API has been broken frequently over the past few years. It used to be enough to say that 3.3 and later were supported (as the README says) but maybe this should be updated. Internally we've used 3.5 most frequently.

But in general this also makes it difficult/obnoxious for new users to try. I've implemented an alternative custom JIT compiler for x86_64 here, just have to connect the two bits and we can drop LLVM entirely.

snow-abstraction commented 4 years ago

Ok. Cool that you've built your own. Is there some discussion somewhere of the trade-offs of using your own versus LLVM? Thanks.

kthielen commented 4 years ago

No there isn't, it's fairly new. The code is in include/hobbes/mc/ and it's a very basic x86_64 JIT (liveness analysis, register allocation, instruction encoding).

I made it to have something much more lightweight than LLVM, to avoid the build issues that people often run into, and the shifting JIT API that causes us to break with each new LLVM version. It's very much specialized just for our use-case, x86_64 JIT compilation. It's not intended to be a general replacement for LLVM.

snow-abstraction commented 4 years ago

Let me know if you don't have time to have this discussion here or if it would be better elsewhere.

Once include/hobbes/mc/ is reliable/settled, why not drop older LLVM versions (except the one needed internally) and only add support for new versions less often? Is there some reason for supporting so many versions? This would allow you to remove some preprocessor complexity.

On Debian (and I assume on Ubuntu), you can install LLVM-6 and LLVM-7 from the standard distribution packages and the LLVM project makes it easy to install LLVM-8 and LLVM-9 for Debian and Ubuntu (http://apt.llvm.org/). Personally, I don't think it is difficult to build hobbes but good that you are thinking of such issues.

kthielen commented 4 years ago

Talking here is OK, or email if you'd rather talk out of band (my profile has my address).

The existing spaghetti code to deal with multiple LLVM versions and their various conflicting JIT APIs came from internal requests (so a version is forced for non-technical reasons). I've never found added value on the other end of these updates, which is frustrating.

I could add support for another LLVM version, probably not a big deal. But I have to prioritize work like that in balance with where the need is greatest (who is asking for LLVM 9? why do they want it? are they a serious user? etc).

The custom JIT compiler is reliable and stable for its use-case so far, but I'd probably have to make a few more optimization passes to make it a general replacement for LLVM in hobbes.

smunix commented 3 years ago

support for llvm@9 and llvm@10 is added with #379

Chaz6 commented 3 years ago

Thanks for closing this. Unfortunately, the current version of LLVM is 11 (2020-10-12) and it still fails to build for me. Should I open a new bug?

[1/188] Building CXX object CMakeFiles/hobbes-test.dir/test/Convert.C.o
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:7:
../include/hobbes/lang/tylift.H:429:37: error: unused parameter 'tenv' [-Werror,-Wunused-parameter]
    static MonoTypePtr type(typedb& tenv) { return Prim::make("unit"); }
                                    ^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
../include/hobbes/util/llvm.H:15:2: error: "I don't know how to use this version of LLVM"
#error "I don't know how to use this version of LLVM"
 ^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
In file included from ../include/hobbes/util/llvm.H:18:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/IR/DerivedTypes.h:20:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:49:
/home/chaz/.local/local/clang+llvm/include/llvm/Support/SwapByteOrder.h:152:13: error: no template named 'enable_if_t' in namespace 'std'
inline std::enable_if_t<std::is_enum<T>::value, T> getSwappedBytes(T C) {
       ~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/SwapByteOrder.h:154:40: error: no template named 'underlying_type_t' in namespace 'std'; did you mean 'underlying_type'?
      getSwappedBytes(static_cast<std::underlying_type_t<T>>(C)));
                                  ~~~~~^~~~~~~~~~~~~~~~~
                                       underlying_type
/opt/rh/devtoolset-9/root/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/type_traits:2149:12: note: 'underlying_type' declared here
    struct underlying_type
           ^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
In file included from ../include/hobbes/util/llvm.H:18:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/IR/DerivedTypes.h:20:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:50:
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:31:28: error: no template named 'remove_reference_t' in namespace 'std'; did you mean 'remove_reference'?
  using UnderlyingT = std::remove_reference_t<T>;
                      ~~~~~^~~~~~~~~~~~~~~~~~
                           remove_reference
/opt/rh/devtoolset-9/root/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/type_traits:1456:12: note: 'remove_reference' declared here
    struct remove_reference
           ^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
In file included from ../include/hobbes/util/llvm.H:18:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/IR/DerivedTypes.h:20:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:50:
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:48:13: error: no member named 'enable_if_t' in namespace 'std'
    T, std::enable_if_t<std::is_pointer<T>::value>> {
       ~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:48:51: error: expected unqualified-id
    T, std::enable_if_t<std::is_pointer<T>::value>> {
                                                  ^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:58:39: error: no member named 'enable_if_t' in namespace 'std'
struct add_const_past_pointer<T, std::enable_if_t<std::is_pointer<T>::value>> {
                                 ~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:58:77: error: expected unqualified-id
struct add_const_past_pointer<T, std::enable_if_t<std::is_pointer<T>::value>> {
                                                                            ^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:68:40: error: no member named 'enable_if_t' in namespace 'std'
                                  std::enable_if_t<std::is_pointer<T>::value>> {
                                  ~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/type_traits.h:68:78: error: expected unqualified-id
                                  std::enable_if_t<std::is_pointer<T>::value>> {
                                                                             ^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
In file included from ../include/hobbes/util/llvm.H:18:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/IR/DerivedTypes.h:20:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/ArrayRef.h:12:
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:104:6: error: no template named 'enable_if_t' in namespace 'std'
std::enable_if_t<is_integral_or_enum<T>::value, hash_code> hash_value(T value);
~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:265:16: error: no matching constructor for initialization of 'llvm::hashing::detail::hash_state'
    hash_state state = {
               ^       ~
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:258:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 7 were provided
struct hash_state {
       ^
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:258:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 7 were provided
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:258:8: note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 7 were provided
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:362:6: error: no template named 'enable_if_t' in namespace 'std'
std::enable_if_t<is_hashable_data<T>::value, T>
~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:370:6: error: no template named 'enable_if_t' in namespace 'std'
std::enable_if_t<!is_hashable_data<T>::value, size_t>
~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:444:6: error: no template named 'enable_if_t' in namespace 'std'
std::enable_if_t<is_hashable_data<ValueT>::value, hash_code>
~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/ADT/Hashing.h:629:6: error: no template named 'enable_if_t' in namespace 'std'
std::enable_if_t<is_integral_or_enum<T>::value, hash_code> hash_value(T value) {
~~~~~^
In file included from ../test/Compiler.C:1:
In file included from ../include/hobbes/hobbes.H:6:
In file included from ../include/hobbes/eval/cc.H:13:
In file included from ../include/hobbes/eval/jitcc.H:9:
In file included from ../include/hobbes/util/llvm.H:18:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/IR/DerivedTypes.h:20:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/ArrayRef.h:14:
In file included from /home/chaz/.local/local/clang+llvm/include/llvm/ADT/SmallVector.h:20:
/home/chaz/.local/local/clang+llvm/include/llvm/Support/MathExtras.h:396:23: error: no template named 'enable_if_t' in namespace 'std'
constexpr inline std::enable_if_t<(N < 64), bool> isUInt(uint64_t X) {
                 ~~~~~^
/home/chaz/.local/local/clang+llvm/include/llvm/Support/MathExtras.h:401:23: error: no template named 'enable_if_t' in namespace 'std'
constexpr inline std::enable_if_t<N >= 64, bool> isUInt(uint64_t X) {
                 ~~~~~^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
[...]
smunix commented 3 years ago

Thanks for closing this. Unfortunately, the current version of LLVM is 11 (2020-10-12) and it still fails to build for me. Should I open a new bug?

The final PR addressing this issue is yet to be merged (#380). After this is merged, the officially supported versions will be the following:

└───packages
    ├───x86_64-darwin
    │   ├───hobbes-llvm-10: package 'hobbes-llvm-10-20210109.02dc46f'
    │   ├───hobbes-llvm-6: package 'hobbes-llvm-6-20210109.02dc46f'
    │   ├───hobbes-llvm-8: package 'hobbes-llvm-8-20210109.02dc46f'
    │   └───hobbes-llvm-9: package 'hobbes-llvm-9-20210109.02dc46f'
    └───x86_64-linux
        ├───hobbes-llvm-10: package 'hobbes-llvm-10-20210109.02dc46f'
        ├───hobbes-llvm-6: package 'hobbes-llvm-6-20210109.02dc46f'
        ├───hobbes-llvm-8: package 'hobbes-llvm-8-20210109.02dc46f'
        └───hobbes-llvm-9: package 'hobbes-llvm-9-20210109.02dc46f'

I have initiated a separate ticket for LLVM-11 support