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.

Sign up for GitHub

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

Jump to bottom

Support $LLVM_VERSION, $LLVM_TARGETS, and $LLVM_LDFLAGS #15091

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Support $LLVM_VERSION, $LLVM_TARGETS, and $LLVM_LDFLAGS #15091

wants to merge 2 commits into from
+27 −13

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Oct 17, 2024 •
edited
Loading

Resolves part of #14376.

This may also address part of #15074 since we can now override LLVM_VERSION for the Makefiles as well.

@straight-shoota
Copy link
Member

I'm wondering about the usefulness of LLVM_LDFLAGS. Effectively, you can just pass extra linker flags via --link-flags. I suppose the only purpose would be to suppress flags from llvm_VERSION and llvm-config --libs. But at that point it might just be reasonable to use neither of them and provide version (and targets) explicitly via env var and specify --link-flags.
This is just an observation, asking if it resonates with aynone. I'm not entirely sure about covering all use cases.

@HertzDevil
Copy link
Contributor Author

On POSIX environments you can probably pass LLVM_CONFIG=true to suppress all the default configuration too (the Makefiles can be easily adjusted for this).

@HertzDevil
Copy link
Contributor Author

Maybe we could drop all those linker flags whenever cross-compiling, but I found that there is no way to tell in Crystal code whether we are also doing the cross-linking on the same host (i.e. --target without --cross-compile), since --cross-compile doesn't add any flags.

@HertzDevil HertzDevil mentioned this pull request Oct 20, 2024
Merged
straight-shoota pushed a commit that referenced this pull request Oct 21, 2024
@HertzDevil
Make Makefile work on MSYS2 (#15102)
b10f171 
* Uses `llvm-config` instead of `llvm_VERSION` for the `x86_64-windows-gnu` target. Also invokes the `find-llvm-config` script using `sh` explicitly and ensures the script returns a Windows path. (This pretty much means a MinGW-w64-based compiler will require the entire MSYS2 environment to work.)
* Increases the stack size from the default 2 MiB to 8 MiB, consistent with builds using the MSVC toolchain.
* `Makefile` and the `bin/crystal` script now recognize `.build/crystal.exe` as the local compiler under MSYS2.
* If `.build/crystal.exe` already exists and we are on Windows, `Makefile` now builds a temporary executable first, just like in `Makefile.win`.
* Ensures `Makefile` doesn't interpret backslashes in `llvm-config.exe`'s path as escape sequences.
* Adds a separate `install_dlls` target for installing the compiler's dependent DLLs to the given prefix's `bin` directory. This is only needed if the installation is to be distributed outside MSYS2.
* Detects the presence of `lld` which can be installed using the `mingw-w64-ucrt-x86_64-lld` package.

With this patch, cross-compiling from MSVC-based Crystal will no longer work until #15091 is merged. Instead it can be done from Linux, including WSL, assuming the host and target LLVM versions are identical (right now MSYS2 has 18.1.8):

```sh
# on Linux
make clean crystal && make -B target=x86_64-windows-gnu

# on MSYS2's UCRT64 environment
pacman -Sy \
  mingw-w64-ucrt-x86_64-gcc mingw-w64-ucrt-x86_64-pkgconf \
  mingw-w64-ucrt-x86_64-gc mingw-w64-ucrt-x86_64-pcre2 mingw-w64-ucrt-x86_64-libiconv \
  mingw-w64-ucrt-x86_64-zlib mingw-w64-ucrt-x86_64-llvm
cc .build/crystal.obj -o .build/crystal.exe \
  $(pkg-config bdw-gc libpcre2-8 iconv zlib --libs) \
  $(llvm-config --libs --system-libs --ldflags) \
  -lDbgHelp -lole32 -lWS2_32 -Wl,--stack,0x800000
```
CTC97 pushed a commit to CTC97/crystal that referenced this pull request Nov 9, 2024
@HertzDevil @CTC97
Make Makefile work on MSYS2 (crystal-lang#15102)
bb217a1 
* Uses `llvm-config` instead of `llvm_VERSION` for the `x86_64-windows-gnu` target. Also invokes the `find-llvm-config` script using `sh` explicitly and ensures the script returns a Windows path. (This pretty much means a MinGW-w64-based compiler will require the entire MSYS2 environment to work.)
* Increases the stack size from the default 2 MiB to 8 MiB, consistent with builds using the MSVC toolchain.
* `Makefile` and the `bin/crystal` script now recognize `.build/crystal.exe` as the local compiler under MSYS2.
* If `.build/crystal.exe` already exists and we are on Windows, `Makefile` now builds a temporary executable first, just like in `Makefile.win`.
* Ensures `Makefile` doesn't interpret backslashes in `llvm-config.exe`'s path as escape sequences.
* Adds a separate `install_dlls` target for installing the compiler's dependent DLLs to the given prefix's `bin` directory. This is only needed if the installation is to be distributed outside MSYS2.
* Detects the presence of `lld` which can be installed using the `mingw-w64-ucrt-x86_64-lld` package.

With this patch, cross-compiling from MSVC-based Crystal will no longer work until crystal-lang#15091 is merged. Instead it can be done from Linux, including WSL, assuming the host and target LLVM versions are identical (right now MSYS2 has 18.1.8):

```sh
# on Linux
make clean crystal && make -B target=x86_64-windows-gnu

# on MSYS2's UCRT64 environment
pacman -Sy \
  mingw-w64-ucrt-x86_64-gcc mingw-w64-ucrt-x86_64-pkgconf \
  mingw-w64-ucrt-x86_64-gc mingw-w64-ucrt-x86_64-pcre2 mingw-w64-ucrt-x86_64-libiconv \
  mingw-w64-ucrt-x86_64-zlib mingw-w64-ucrt-x86_64-llvm
cc .build/crystal.obj -o .build/crystal.exe \
  $(pkg-config bdw-gc libpcre2-8 iconv zlib --libs) \
  $(llvm-config --libs --system-libs --ldflags) \
  -lDbgHelp -lole32 -lWS2_32 -Wl,--stack,0x800000
```
@HertzDevil HertzDevil mentioned this pull request Nov 15, 2024
Merged
straight-shoota pushed a commit that referenced this pull request Nov 18, 2024
@HertzDevil
Use MSYS2's upstream LLVM version on MinGW-w64 CI (#15197)
5e02cd4 
MSYS2's Pacman has upgraded LLVM to 19.1.3-1 and dropped 18, breaking our CI, so we pick a newer LLVM version on the cross-compiling host to match the target for now (i.e. until #15091)

This should fix CI failures on MSYS2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
kind:feature topic:stdlib:llvm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@HertzDevil @straight-shoota