GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-33351: (WIP) Patches to build on clang-cl by emmatyping · Pull Request #7680 · python/cpython

Conversation

@emmatyping

@emmatyping emmatyping commented Jun 13, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I've been a bit busy so I haven't had time to prepare this, but I wanted to post something people can try, so here's something.

So far I have made changes to:

  • forward declare struct timeval in pytime (it should have been anyway)
  • move some struct packing in the correct place
  • Using a more portable stringize macro
  • defining compiler names

This builds and passes tests with MSVC, and clang-cl on x64.

If you would like to test this:

  • Grab a Windows snapshot build of clang trunk from here: http://llvm.org/builds/ or build clang from source (not recommended, this is quite time, processor, and memory intensive).
    then
  • Change project files to build with the LLVM-2014 toolset (I haven't figured out a good way to do this automatically yet).

Everything builds except:

  • _decimal (clang-cl doesn't like linking mpd it seems)
  • _distutils_findvs (unsurprisingly)
  • pyshellext (it doesn't like the idl file)

The good news is that clang generates MSVC ABI compatible binaries, so linking clang/msvc binaries together is fine.

The following tests fail under clang-cl:

test_compile test_compileall test_ctypes test_exceptions
test_fileio test_io test_json test_os test_pickle test_pickletools
test_richcmp test_runpy test_script_helper test_sys test_traceback
test_userdict test_userlist test_xml_etree

It seems many of them are failing with a stack overflow, so it seems clang is miscompiling something.

There is however a subset that can compile under clang while still passing the tests.

https://bugs.python.org/issue33351

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@emmatyping
Patches to build on clang-cl
fe7e8bf 
This was mostly:
- forward declare struct timeval in pytime (it should have been anyway)
- moving struct packing in the correct place
- Using a more portable, standard garunteed stringize macro
- defining compiler names
@emmatyping
emmatyping requested a review from a team as a code owner June 13, 2018 10:29
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jun 13, 2018
@bedevere-bot bedevere-bot added the awaiting review label Jun 13, 2018
@emmatyping emmatyping mentioned this pull request Jun 13, 2018
bpo-33351: Correct struct packing in _tracemalloc.c #6761
Closed
emmatyping added 2 commits June 13, 2018 10:53
@emmatyping
Fix whitespace issue in pytime.h
48ff92e
@emmatyping
Add space in compiler version
a61dad1
@ahartikainen ahartikainen mentioned this pull request Jul 10, 2018
Appveyor setup - msvc + clang-cl stan-dev/httpstan#59
Closed
@ahartikainen

ahartikainen commented Jul 17, 2018

Copy link
Copy Markdown

Should this PR also add clang-cl compiler option for distutils?
clang-cl could basically use a copy of the MSVCCompiler class in _msvccompiler.py

It would only need to change line 245: self.cc = _find_exe("cl.exe", paths) to self.cc = _find_exe("clang-cl.exe", paths). Also some of the compiler arguments are unnecessary.

Also line 960 compiler_class in ccompiler.py needs updating.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@emmatyping

emmatyping commented Jul 17, 2018

Copy link
Copy Markdown
Member Author

Should this PR also add clang-cl compiler option for distutils?

@ahartikainen I think that is likely the next todo I have. I'll get back to this soon hopefully.
ahartikainen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Apr 12, 2019

Copy link
Copy Markdown
Member

@ethanhs Are you still interested in pushing this forward? If so, can you merge against master and run CI again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@emmatyping

emmatyping commented Apr 12, 2019

Copy link
Copy Markdown
Member Author

@zooba, yes! I have been working on other things recently, but it would be helpful to know what a mergable PR should look like. Since clang-cl can fallback to cl, there may be a few modules I will need to use that for, as mentioned in the PR comment there seems to be an issue with how it compiles things (however I should test this with a newer clang to be sure).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Apr 12, 2019

Copy link
Copy Markdown
Member

Of the three modules that didn't build, distutils_findvs has been removed and pyshellext is probably optional (it's part of the py.exe launcher, not Python itself). So that will get you closer.

You should be able to pass /p:PlatformToolset="whatever" on the command line (or set it as an environment variable) to override the toolset when building outside of VS. The "right" way to do in within VS is to add a new configuration, but that's far more intrusive (going to have to touch every project file and test it extensively).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zufuliu

zufuliu commented May 30, 2019

Copy link
Copy Markdown

Add clang-cl to distutils is just a few lines of change, I added a patch at https://github.com/zufuliu/llvm-utils/blob/master/clang/clang-cl-py3.diff, which works fine for 3.7.3.
ahartikainen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@isuruf isuruf mentioned this pull request Dec 1, 2019
ENH: enable multi-platform SIMD compiler optimizations numpy/numpy#13516
Merged
@isuruf isuruf mentioned this pull request Feb 6, 2020
bpo-33351: Patches to build on clang-cl #18371
Closed
@csabella

csabella commented Feb 6, 2020

Copy link
Copy Markdown
Contributor

This has been replaced by GH-18371.
emmatyping reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella csabella closed this Feb 6, 2020
@ethanhs ethanhs mannequin mentioned this pull request Aug 4, 2022
Support compiling with clang-cl on Windows #77532
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@emmatyping @ahartikainen @zooba @zufuliu @csabella @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.