GH-131278: Support building using "computed gotos" for clang-cl on Windows by chris-eibl · Pull Request #131279 · python/cpython · GitHub

For details please see #131278.

Issue: Support building using "computed gotos" for clang-cl on Windows #131278

clang-cl on Windows. Patch by Chris Eibl.

Some readability comments. Do you want to actually add a What's New as well? (I don't know how wide is the audience)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

I don't remember if we had a Whats New for the original clang-cl issue. We should definitely mention in the whats new that clang cl building on Windows is now possible, and say PGO too.

Woops sorry pressed the wrong button!

@chris-eibl How do you plan to address:

We should definitely mention in the whats new that clang cl building on Windows is now possible, and say PGO too.

Are there more PRs for those kind of optimizations on Windows? if so, we can delay everything until they are done. Otherwise, let's mention that in the What's New entry

I don't remember if we had a Whats New for the original clang-cl issue. We should definitely mention in the whats new that clang cl building on Windows is now possible, and say PGO too.

Building with clang-cl has been possible for a very long time: #101352.

The only fixes I am aware of are just these two (and just main and current 3.14 alphas are/have been affected):

_GENERATE_DEBUG_SECTION_WINDOWS https://github.com/python/cpython/pull/130040/files#r1981801677 already fixed by you
blake2module.c needs to be compiled with libhacl SIMD flags #130213: close to being fixed

So maybe just mention PGO in the Whats New?
Maybe this here and -flto=thin, too?

@zooba treats clang-cl as quite optional in #129907 (comment)
but I think a Whats New entry would not hurt.

Shall I give it a go here and mention those three things?

@Fidget-Spinner : your tail call for Windows #130040 (comment) came without What's new, too. Shall I add an entry, too?

Are there more PRs for those kind of optimizations on Windows?

I'd like to work on the many warnings clang-cl emits, but IMHO that won't go into What's new. Regarding optimizations, I think that's it atm, we can amend later, anyway.

@Fidget-Spinner : your tail call for Windows #130040 (comment) came without What's new, too. Shall I add an entry, too?

We don't need it, as there's already a whats new for tail call in general.

IMHO "computed gotos and/or tail calls" would be better here, but up to @Fidget-Spinner 's liking :)

I think we want to be careful putting too much into What's New, since we still aren't technically supporting clang-cl on Windows yet, we're really just unblocking it. No doubt if @chris-eibl sticks around it'll remain unblocked, but also, if Clang one day breaks down and MSVC is fine then we're still going to release.

A simple entry (which should already be there?) stating that build.bat now has a clang-cl specific option (previously you needed to set PlatformToolset but that was the only difference) and perhaps calling out that this enables some clang-specific optimisations, referencing PCbuild\readme.txt for more info seems to hit the balance. Let's avoid using the word "support" in regards to our source code.

previously you needed to set PlatformToolset but that was the only difference

This is still true, we do not have a "clang" switch for build.bat, yet.

I am fine with entirely removing the whatsnew if that makes it easier :)

PCbuild changes look good, just suggesting some clarifications to the docs.

Suggested change

      
            Add optimizing flag ``WITH_COMPUTED_GOTOS`` to support such builds using
          
            clang-cl on Windows. Patch by Chris Eibl.
          
            Add optimizing flag ``WITH_COMPUTED_GOTOS`` to Windows builds for when
          
            using a compiler that supports it (currently clang-cl). Patch by Chris Eibl.

Suggested change

      
            * building with clang-cl on Windows now supports PGO (profile guided
          
            * clang-cl builds on Windows now work with ``--pgo`` (profile guided

Suggested change

      
              optimization), uses ``-flto=thin`` and can be configured to use
          
              optimization), and can be configured to use

Is using -flto=thin important to call out? It shouldn't change the ABI of the build output at all, and it's not configurable.

This is still true, we do not have a "clang" switch for build.bat, yet.

(Seen after submitting my review.) Oh don't we? Okay, yeah, let's treat all clang-cl specific changes as undocumented then. The day it gets an option is when we'd start putting notes in the Build section of NEWS, and give the option a brief mention in What's New.

This reverts commit 1f5b55f.

…ith clang-cl (pythonGH-131279)

Add optimizing flag WITH_COMPUTED_GOTOS to support such builds using

98de4c1

clang-cl on Windows. Patch by Chris Eibl.

chris-eibl requested a review from a team as a code owner March 15, 2025 11:35

bedevere-app Bot added the awaiting review label Mar 15, 2025

bedevere-app Bot mentioned this pull request Mar 15, 2025

Support building using "computed gotos" for clang-cl on Windows #131278

Closed

ups - double backticks needed

86cb64e

picnixz reviewed Mar 15, 2025

View reviewed changes

Comment thread PCbuild/readme.txt

Comment thread PCbuild/readme.txt Outdated

Comment thread PCbuild/readme.txt Outdated

Comment thread Misc/NEWS.d/next/Build/2025-03-15-12-32-56.gh-issue-131278.1nd0mJ.rst Outdated

Apply suggestions from code review

3aabeea

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Fidget-Spinner closed this Mar 15, 2025

Fidget-Spinner reopened this Mar 15, 2025

picnixz reviewed Mar 15, 2025

View reviewed changes

Comment thread PCbuild/readme.txt Outdated

picnixz reviewed Mar 15, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Build/2025-03-15-12-32-56.gh-issue-131278.1nd0mJ.rst Outdated

chris-eibl added 2 commits March 15, 2025 13:36

remove space before *

ca22c81

revert :prog:

897c527

picnixz approved these changes Mar 15, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 15, 2025

add whats new

1f5b55f

chris-eibl commented Mar 15, 2025

View reviewed changes

zooba reviewed Mar 17, 2025

View reviewed changes

chris-eibl added 2 commits March 17, 2025 18:52

Revert "add whats new"

905d2e4

This reverts commit 1f5b55f.

apply Steve's suggestion to blurb

fd3cb86

fix missing newline in blurb

59dac20

zooba merged commit 468a7aa into python:main Mar 17, 2025

bedevere-app Bot removed the awaiting merge label Mar 17, 2025

chris-eibl deleted the param_cg branch March 18, 2025 05:25

colesbury pushed a commit to colesbury/cpython that referenced this pull request Mar 20, 2025

pythonGH-131278 : Add option to build with computed gotos on Windows w…

ca39b4f

…ith clang-cl (pythonGH-131279)

chris-eibl mentioned this pull request Aug 23, 2025

GH-131033: Enable the optimizing macros UNLIKELY and LIKELY for clang-cl on Windows #131019

Merged

		Add optimizing flag ``WITH_COMPUTED_GOTOS`` to support such builds using
		clang-cl on Windows. Patch by Chris Eibl.

                 with :c:expr:`Py_NO_LINK_LIB`. (Contributed by Jean-Christophe
                 Fillion-Robin in :gh:`82909`.)
+              * building with clang-cl on Windows now supports PGO (profile guided

	* building with clang-cl on Windows now supports PGO (profile guided
	* clang-cl builds on Windows now work with ``--pgo`` (profile guided

	optimization), uses ``-flto=thin`` and can be configured to use
	optimization), and can be configured to use

GH-131278: Support building using "computed gotos" for clang-cl on Windows by chris-eibl · Pull Request #131279 · python/cpython · GitHub

Uh oh!

Uh oh!

GH-131278: Support building using "computed gotos" for clang-cl on Windows - #131279

GH-131278: Support building using "computed gotos" for clang-cl on Windows#131279zooba merged 9 commits intopython:mainpython/cpython:mainfrom chris-eibl:param_cgchris-eibl/cpython:param_cgCopy head branch name to clipboard

Conversation

chris-eibl commented Mar 15, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fidget-Spinner commented Mar 15, 2025

Uh oh!

Fidget-Spinner commented Mar 15, 2025

Uh oh!

Uh oh!

Uh oh!

picnixz commented Mar 15, 2025

Uh oh!

chris-eibl commented Mar 15, 2025

Uh oh!

chris-eibl commented Mar 15, 2025

Uh oh!

Fidget-Spinner commented Mar 15, 2025

Uh oh!

chris-eibl Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

zooba commented Mar 17, 2025

Uh oh!

chris-eibl commented Mar 17, 2025

Uh oh!

zooba left a comment

Choose a reason for hiding this comment

Uh oh!

zooba Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

zooba Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

zooba Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

zooba commented Mar 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GH-131278: Support building using "computed gotos" for clang-cl on Windows#131279
zooba merged 9 commits into
python:mainfrom
chris-eibl:param_cg

chris-eibl commented Mar 15, 2025 •
edited by bedevere-app Bot
Loading