Oh woops I thought I was looking at a different pr!

The simpler formulation

inst(LOAD_SPECIAL, (self--method_and_self[2])) {method_and_self[1]=self;method_and_self[0]=NULL;DEAD(self);PyObject*name=_Py_SpecialMethods[oparg].name;interr=_PyObject_LookupSpecialMethod(name,method_and_self);if (err<0) {if (!_PyErr_Occurred(tstate)) {_PyErr_Format(tstate,PyExc_TypeError,_Py_SpecialMethods[oparg].error,PyStackRef_TYPE(method_and_self[1])->tp_name);ERROR_NO_POP();                }ERROR_NO_POP();            }        }

should work now, with no changes to the cases generator needed.

All commit authors signed the Contributor License Agreement.

@markshannon@brandtbucher - the JIT build is failing on this PR. Would you please take a look? I think the generatedoptimizer_cases.c.h incorrectly manipulates the stack pointer.

Thanks for the ping, I’ll take a look at this tomorrow.

Just from looking at the diff, Ithink the issue may be that we need a case inoptimizer_bytecodes.c for the new_INSERT_NULL instruction, that sets the output to a null symbol (usingsum_new_null, see_PUSH_NULL for an example). The default implementation that’s generated assumes all outputs are non-NULL, which obviously isn’t true here. I can confirm that this is the issue in a bit.

Thanks@brandtbucher! Would you please take a look at my changes tooptimizer_bytecodes.c to see if they make sense?

I don't see the need for arrays. Using arrays forces values into memory which prevents top of stack caching, and it makes the code harder to follow.

If you give the_PyObject_LookupSpecialMethod the signatureint _PyObject_LookupSpecialMethod(PyObject *owner, PyObject *name, _PyStackRef *method); and have it return -1 for error, 0 for normal attribute and 1 for method, then you can use the simpler implementation below. It is also necessary for_PyObject_LookupSpecialMethod tonot steal a reference toowner, but that's an easy change.

Using scalars works, as the code generator can track what is live or not and make sure that everything live is visible to the GC.
At least, it should do. Please file a bug report if it doesn't.

This does what you want, I think:

inst(LOAD_SPECIAL, (owner--attr,self_or_null)) {PyObject*owner_o=PyStackRef_AsPyObjectBorrow(owner);PyObject*name=_Py_SpecialMethods[oparg].name;// Returns -1 for error, 0 for normal attr, 1 for method.interr=_PyObject_LookupSpecialMethod(owner_o,name,&attr);if (err>0) {self_or_null=owner;DEAD(owner);            }elseif (err<0) {// The code generator assumes that _PyObject_LookupSpecialMethod// has defined `attr`, so overwrite it with `owner`.attr=owner;DEAD(owner);if (!_PyErr_Occurred(tstate)) {_PyErr_Format(tstate,PyExc_TypeError,_Py_SpecialMethods[oparg].error,Py_TYPE(owner_o)->tp_name);                }ERROR_IF(true,error);            }else {PyStackRef_CLOSE(owner);self_or_null=PyStackRef_NULL;            }        }

When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

No, that doesn't work@markshannon. Bothowner and the looked up method need to be on the stack and visible to the GC.

In your formulation, once_PyObject_LookupSpecialMethod() writes the method to&attr, it will overwrite the stack slot containing owner. That's a problem because the GC will not seeowner in a subsequent escaping call (such as thetp_descr_get call in_PyObject_LookupSpecialMethod).

Here is part of the generated code for your formulation._PyObject_LookupSpecialMethod() needs to store the looked up method/attribute in a place that's visible to the GC. The&attr is not visible to the GC because it's just a local C variable.

_PyStackRefowner;_PyStackRefattr;_PyStackRefself_or_null;owner=stack_pointer[-1];PyObject*owner_o=PyStackRef_AsPyObjectBorrow(owner);PyObject*name=_Py_SpecialMethods[oparg].name;_PyFrame_SetStackPointer(frame,stack_pointer);interr=_PyObject_LookupSpecialMethod(owner_o,name,&attr);

attr is not defined when_PyObject_LookupSpecialMethod is called, so it would be bad if the GC could see it.

That's why it's initialized toPyStackRef_NULL in this PR, but you can't do that with your formulation.

No difference in single-threaded performance (<0.02% default; <0.06% FT; clang-20 tail-call)

🤖 New build scheduled with the buildbot fleet by@colesbury for commit270b87b 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131851%2Fmerge

If you want to schedule another build, you need to add the🔨 test-with-refleak-buildbots label again.

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotaarch64 Fedora Stable Clang 3.x (tier-2) has failed when building commitda53660.

What do you need to do:

1. Don't panic.
2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/234/builds/7453) and take a look at the build logs.
4. Check if the failure is related to this commit (da53660) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/234/builds/7453

Failed tests:

• test_interpreters

Failed subtests:

• test_python_calls_do_not_appear_in_the_stack_if_perf_deactivated - test.test_perf_profiler.TestPerfProfilerWithDwarf.test_python_calls_do_not_appear_in_the_stack_if_perf_deactivated

Summary of the results of the build (if available):

==

Traceback (most recent call last):  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1079, in_bootstrap_innerself._context.run(self.run)~~~~~~~~~~~~~~~~~^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1021, inrunself._target(*self._args,**self._kwargs)~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line30, intask    interp= interpreters.create()  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line76, increateid= _interpreters.create(reqrefs=True)interpreters.InterpreterError:interpreter creation failedkTraceback (most recent call last):  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1079, in_bootstrap_innerself._context.run(self.run)~~~~~~~~~~~~~~~~~^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/threading.py", line1021, inrunself._target(*self._args,**self._kwargs)~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_interpreters/test_stress.py", line47, inrun    interp= interpreters.create()  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/support/interpreters/__init__.py", line76, increateid= _interpreters.create(reqrefs=True)interpreters.InterpreterError:interpreter creation failedkTraceback (most recent call last):  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.clang/build/Lib/test/test_perf_profiler.py", line401, intest_python_calls_do_not_appear_in_the_stack_if_perf_deactivatedself.assertEqual(stderr,"")~~~~~~~~~~~~~~~~^^^^^^^^^^^^AssertionError:'Warning:\nProcessed 259 events and lost 1[34 chars]\n\n' != ''- Warning:- Processed 259 events and lost 1 chunks!- - Check IO/CPU overload!-