gh-148646: Add --enable-prebuilt-jit-stencils configure flag#148647
gh-148646: Add --enable-prebuilt-jit-stencils configure flag#148647hroncok wants to merge 1 commit into
Conversation
|
As is, this solves my problem. Happy to bikeshed the option names. |
|
Converted back to draft to rebase as there were conflicts with 9633c52 |
9d17d1d to
1f0c38e
Compare
|
This works for us with 3.15.0b1. |
savannahostrowski
left a comment
There was a problem hiding this comment.
I think there are actually two distinct problems tangled up here, and it's worth separating them:
Problem 1: the inputs to the digest are too coarse. This isn't trivial to narrow and probably deserves its own exploration — I've opened #150320 to track that separately.
Problem 2: Karolina's aarch64 case (Discuss). If LLVM patch versions produce textually different but semantically equivalent stencils, then no amount of input-hashing will catch that. We could be stricter about the patch version we support for stencils but I don't really think that's necessary at the moment.
In my mind, this PR should be the fix for problem 2, although I know it's an escape hatch for problem 1 as well, until we figure that out.
In any case, I'm going to bikeshed the flag name here. I have a slight preference for --with-jit-stencils[=path] for consistency with other CPython --with-*=PATH flags like --with-build-python or --with-openssl. For your workflow it'd be functionally identical but it'd give folks flexibility on where they store their stencils.
| outline = "=" * len(warning) | ||
| print("\n".join(["", outline, warning, request, outline, ""])) | ||
| digest = f"// {self._compute_digest()}\n" | ||
| stencils_current = ( |
There was a problem hiding this comment.
If prebuilt=True but the stencil file doesn't exist, stencils_current is False and we fall through to _build_stencils(). It's probably safer to raise in this case instead of silently building fresh stencils despite the user explicitly asking us to use prebuilt ones.
| [accept prebuilt JIT stencil headers even if the digest does not match (default is no)])], | ||
| [], | ||
| [enable_prebuilt_jit_stencils=no]) | ||
| AS_VAR_IF([enable_prebuilt_jit_stencils], |
There was a problem hiding this comment.
This appends --prebuilt to REGEN_JIT_COMMAND even when JIT is disabled. We should probably nest the AS_VAR_IF block inside the AS_VAR_IF([jit_flags], ...) block above so it only applies when JIT is enabled, and also error out if --enable-prebuilt-jit-stencils is set without --enable-experimental-jit.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Uh oh!
There was an error while loading. Please reload this page.