Skip to content

ENH: Add support for building with clang-cl #13816

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

Closed
wants to merge 21 commits into from

Conversation

bashtage
Copy link
Contributor

@bashtage bashtage commented Jun 21, 2019

Add support for clang-cl on Windows

NumPy builds and passes with clang-cl.

  • Is this a worthwhile effort? Is it useful? I have found clang-cl to be very helpful in some situations where MSVC won't produce decent code.
  • Are there a lot of MSVC-specific changes that need to be checked?
  • If this goes forward, should there be a CI build with it?
  • Need to check x86 as well.
  • Does this need to go to the mailing list?

cc @matthew-brett

@charris
Copy link
Member

charris commented Jun 23, 2019

I think this is useful, I have hopes that clang will eventually give us opensource C and Fortran compilers for windows, especially the latter.

@mattip
Copy link
Member

mattip commented Jul 7, 2019

Can you compile cpython with clangcl on windows? I am a bit surprised we need to add our own ClangCL class to distutils.

It would indeed be useful to set up a windows CI run under 32-bit and 64-bit for this.

@bashtage
Copy link
Contributor Author

bashtage commented Jul 7, 2019

Can you compile cpython with clangcl on windows? I am a bit surprised we need to add our own ClangCL class to distutils.

There is an open ticket with a patch to allow it to build: https://bugs.python.org/issue33351

This said, distutils doesn't support clang-cl any more than distutils supports icc on windows. It is unfortunate that commands like CC={cl|icl|clang-cl} python setup.py develop don't work Windows like that do on Unix.

AKAICT this is why NumPy would need to directly support clang-cl (if it is a good idea).

@mattip
Copy link
Member

mattip commented Jul 16, 2019

How would this play out for clang on linux, since the new class is windows-specific?

@bashtage
Copy link
Contributor Author

clang on unix is trivial since you can use cc=clang python setup.py install. Windows doesn't honor cc variables.

@mattip
Copy link
Member

mattip commented Jul 16, 2019

What is the magical incantation to get runtests.py or setup.py to use clang on windows?

@bashtage
Copy link
Contributor Author

I used the same trick that is used for icc

python setup.py config --compiler=clang-cl build_clib --compiler=clang-cl build_ext --compiler=clang-cl install

See https://software.intel.com/en-us/articles/numpyscipy-with-intel-mkl

@bashtage
Copy link
Contributor Author

It is likely that I'm not doing it correctly.

@mattip
Copy link
Member

mattip commented Sep 17, 2019

build already accepts a compiler option and passes it on to the sub-commands, so it should be sufficient to run setup.py build --compiler=clang-cl. We could add a --compiler option to runtests.py that would get passed on to build

@bashtage
Copy link
Contributor Author

That would probably help. I think clang-cl might still need a few settings since it isn't 100% identical to cl.

@serge-sans-paille
Copy link
Contributor

@bashtage : I <3 that PR. If you're interested in extra tests, I can launch the full pythran recompilation based on this, we are in great need for such support, and currently rely on a dirty hack for clang-cl support.

@bashtage
Copy link
Contributor Author

bashtage commented Dec 3, 2019

I would like to finish this in the next month or so. Needs a good bit of cleaning I think.

@h-vetinari
Copy link
Contributor

Any news on this PR? :)

@h-vetinari
Copy link
Contributor

I hope another ping 4 months later does not constitute bad form. 😉

@bashtage bashtage force-pushed the clang-windows-build branch from 5f1d7dc to 6b0aab5 Compare May 28, 2020 21:27
@bashtage
Copy link
Contributor Author

@mattip I've updated to reflect changes and this is working. I have also improved it. Any idea how I could take this forward?

@bashtage bashtage force-pushed the clang-windows-build branch 5 times, most recently from ac28f73 to c79ee85 Compare May 29, 2020 16:25
@bashtage
Copy link
Contributor Author

I think it is there, although ideally it would get a CI run.

python runtests.py --compiler=clang-cl
.....................................................................................
.....................................................................................
.....................................................................................
10394 passed, 454 skipped, 108 deselected, 17 xfailed, 1 xpassed in 73.10s (0:01:13)     

@bashtage bashtage changed the title WIP/ENH: Add support for building with clang-cl ENH: Add support for building with clang-cl May 29, 2020
bashtage and others added 20 commits January 20, 2022 18:06
Add support for clang-cl on Windows
Fix compiler for recent changes in NumPy
Add support for --compiler in run tests
Clarify methods to build or test
Make definition and implementation match exactly for clang-cl compat
Forbid compiling 32 with 64 bit clang-cl and vice versa
Link against built-ins on 32-bit platforms
Improve formatting and add comments
Correct == incorrectly changed to not in
Co-authored-by: Matti Picus <matti.picus@gmail.com>
Fix for inlining issue
Try lld-link
Add optimization
Revert changes for warnings awaiting numpy#18432
Small clean up of commented code
_udiv128 is not available in clang-cl
Revert change from llabs to labs
Cast to silence warning
Fix type in compiler_type check
Remove check which isn't needed when calling setup with arguments
in the correct order
@bashtage bashtage force-pushed the clang-windows-build branch from 47c0516 to 4c31368 Compare January 20, 2022 18:08
bashtage added a commit to bashtage/numpy that referenced this pull request Jan 20, 2022
Port over changes needed to compile NumPy using clang-cl
from numpy#13816. Desirable to retain these even though distutils is
effectively deprecated and so not worth the effort to alter.
@bashtage bashtage force-pushed the clang-windows-build branch from 4c31368 to b445c57 Compare January 20, 2022 18:17
Bug continues on clang-cl
@bashtage bashtage force-pushed the clang-windows-build branch from b445c57 to df97b04 Compare January 20, 2022 18:39
@InessaPawson
Copy link
Member

Do we wish to continue working on this PR, given numpy.distutils has been officially deprecated?

@bashtage
Copy link
Contributor Author

The PR is eol. I think the good parts all got merged in other PRs.

@mattip
Copy link
Member

mattip commented Mar 1, 2022

Let's close this

@mattip mattip closed this Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants