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

Drop LLVM 3.5 and LLVM 3.6 support #4396

Closed

Conversation

ysbaddaden
Copy link
Contributor

Since omnibus will be updated to use LLVM 3.8.1, this drops support for older, buggy, LLVM releases. This also cleans up the codebase.

The supported LLVM versions are now 3.8, 3.9 and 4.0 (manually tested).

@bcardiff
Copy link
Member

Note: We are missing invocations to llvm-config-4.0, in some linux packages there might be no llvm-config but llvm-config-4.0. (in the Makefile and lib_llvm.cr). I was expecting to do that soon, but it will conflict slightly with this.

This can't be merged until the omnibus-crystal is migrated to use newer llvm, so avoid merging until @matiasgarciaisaia or @waj says so.

@dmitryrck
Copy link

How about Dockerfile? It is using 3.5.0.

%  docker run --rm crystallang/crystal crystal --version
Crystal 0.22.0 [3c71228] (2017-04-20) LLVM 3.5.0

P. S.: I mean not necessarily updating it in this PR, but it is required to update 😉 .

@ysbaddaden
Copy link
Contributor Author

The docker image is based on released Crystal AFAIK so it should be updated on next release.

I initially didn't had llvm-config-4.0 when I merged the compatibility commits, because LLVM4 was in beta. We can decide to upgrade to LLVM4 by default (or as a fallback) now that it's stable. Your call, I'll make the change.

@dmitryrck
Copy link

Great! :D

The supported LLVM versions are 3.8, 3.9 and 4.0.
@ysbaddaden ysbaddaden force-pushed the drop-llvm-35-and-36-support branch from 56e1b8c to 09c8ca9 Compare May 17, 2017 07:31
@akzhan
Copy link
Contributor

akzhan commented May 22, 2017

I suppose that latest stable LLVM should be default one.

@ysbaddaden
Copy link
Contributor Author

LLVM 3.9 is the current stable. LLVM 4 is qualification and LLVM 5 is unstable. Some package managers (i.e. homebrew) are pushing the qualification release as stable but it's actually not by LLVM terms.

@akzhan
Copy link
Contributor

akzhan commented Jun 13, 2017

Looks like these functions should be uncommented.

https://github.com/crystal-lang/crystal/blob/master/src/math/libm.cr#L6

@ysbaddaden
Copy link
Contributor Author

  1. this PR drops support for older LLVM releases (cleanup);
  2. that depends on which LLVM release the intrinsics were introduced (it's unclear).

@akzhan
Copy link
Contributor

akzhan commented Jun 13, 2017

@ysbaddaden As @RX14 said - minnum and maxnum were introduced in LLVM 3.6.

@ysbaddaden
Copy link
Contributor Author

Bump. Since omnibus now relies on LLVM 3.8 can we drop support for older, buggy, LLVM versions?

@ysbaddaden ysbaddaden mentioned this pull request Aug 11, 2017
@sdogruyol
Copy link
Member

Bump 👍

@matiasgarciaisaia
Copy link
Member

matiasgarciaisaia commented Aug 13, 2017 via email

@akzhan
Copy link
Contributor

akzhan commented Aug 14, 2017

Everywhere in code I see mentions about ugly earliest LLVM < 3.9.

We must drop support for old LLVMs because of

  1. LLVM LexicalScopes build exception when building with --release flag #4719 (comment)
  2. Crystal still in early development stages, so support for old codebase is not needed.

@RX14
Copy link
Contributor

RX14 commented Aug 14, 2017

The solution is to statically compile LLVM into the crystal compiler so we can support 1 or maybe 2 LLVM major versions and not have to worry about distributions.

@ysbaddaden
Copy link
Contributor Author

Closing, this was merged in #4821.

@ysbaddaden ysbaddaden closed this Sep 9, 2017
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.

None yet

8 participants