GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||
GeezerVison is showing github.com — GH-132508: Use tagged integers on the evaluation stack for the last instruction offset by markshannon · Pull Request #132545 · python/cpython Conversation
|
|
As expected, performance is neutral. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I believe the magic number needs to be updated (and everything regenerated too) since the exception_unwind label now pushes tagged ints.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
When you're done making the requested changes, leave the comment: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Nothing has changed in terms of stack effects. The only change is that we push a tagged int instead of a boxed one. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The failure on JIT/ARM64 look like a JIT bug. The assertion errors seem impossible and do not occur on any other platform. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I think I've seen that JIT failure before on my old PR when I used a different tagging scheme. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
The JIT ARM 64 windows failure is just a timeout for the int repr test that sometimes happens. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This isn't a JIT problem, it's a tier two problem (I can reproduce on an M2 Mac with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
When entering tier two, the current frame's |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Never mind, it looks like the bug is in the JIT code. I believe this is because the Clang is not actually inlining the some |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@markshannon, this fixes it: diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index 8faa9e8cac2..639e4bcc793 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -291,6 +291,7 @@ def process_relocations( hole.kind in {"R_AARCH64_CALL26", "R_AARCH64_JUMP26", "ARM64_RELOC_BRANCH26"} and hole.value is HoleValue.ZERO + and hole.symbol not in self.symbols ): hole.func = "patch_aarch64_trampoline" hole.need_state = True |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
For anyone curious: on this particular build, Clang doesn't inline one of the calls to Though they both should be identical, my guess is that Clang realized that the function was static and only called in one place by the bytecode, and didn't use an ABI-conforming calling convention. If we change the logic to check for duplicate local symbols in the template before trying to link to the main executable (which is what my change does), then we end up calling the intended version of the function. (And, in case anyone was wondering: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@markshannon, you applied my patch to the wrong line. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| PyObject *obj = _Py_stackref_get_object(ref); | ||
| Py_INCREF(obj); | ||
| return _Py_stackref_create(obj, filename, linenumber); | ||
| if (ref.index & 1) { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| if (ref.index & 1) { | |
| if (is_tagged_int(ref)) { |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.