-
Notifications
You must be signed in to change notification settings - Fork 8
Fix #326: Add a checkbox to build with clang #327
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| pgo: | ||
| description: "Build with PGO" | ||
| type: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what effect does removing this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Github has a limit of only 10 options on a workflow-dispatch trigger (that's the thing that makes a drop down of options in the web interface). In order to accommodate one more checkbox for clang, I had to remove this one. We never should be benchmarking without PGO anyway, so I thought this was the safest to remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok that makes sense. Thanks! I think the next thing to remove if we run out of options again would probably be the tier 2 interpreter since at this point we probably don't care about its performance.

This adds a checkbox to build CPython with "latest" clang, and then will flag the results so we know it's a special configuration.
This only works with macOS and Linux right now. Windows is possible but I plan to do it as a follow-on since it's a bit more involved.
Cc: @Fidget-Spinner, @brandtbucher