Why do we usePyStackRef_IsExactly here (which doesn't mask out the deferred bit) but usePyStackRef_IsFalse (which does mask out the deferred bit) in_POP_JUMP_IF_FALSE above? Is this the rare case where it's safe?

Our codegen ensures that these ops only see True or False. That's often by adding aTO_BOOL immediately before, which may befolded intoCOMPARE_OP. The precedingTO_BOOL, including inCOMPARE_OP, ensures the canonical representation ofPyStackRef_False orPyStackRef_True with the deferred bit set.

However, there are two places incodegen.c that omit theTO_BOOL because they have other reasons to know that the result is exactly a boolean:

TheCOMPARE_OPs here still generate bools, but not always in the canonical representation. So we can either:

1. ModifyCOMPARE_OP to ensure the canonical representation likehttps://github.com/colesbury/cpython/blob/5583ac0c311132e36ef458842e087945898ffdec/Python/bytecodes.c#L2409-L2416
2. UsePyStackRef_IsFalse (instead ofPyStackRef_IsExactly) in theJUMP_IF_FALSE
3. Modify the codegen by insertingTO_BOOL in those two spots.

That makes sense, thanks for the explanation. Since usingPyStackRef_IsExactly safely is sensitive to code generation changes, I might suggest using it only when we're sure it actually matters for performance, and default to using the variants that mask out the deferred bits everywhere by default since those are always safe. I'd guess that this wouldn't affect the performance improvement of this change much, since it should come from avoiding the tagging in_PyStackRef_FromPyObjectSteal. I don't feel super strongly though.

I'll switch to usingPyStackRef_IsFalse andPyStackRef_IsTrue.

I'm no longer convinced thatPyStackRef_IsExactly is actually a performance win (and I didn't see it in measurements). I think we have issues with code generation quality that we'll need to address later. Things likePOP_JUMP_IF_NONE are composed of_IS_NONE and_POP_JUMP_IF_TRUE and we pack the intermediate result in a tagged_PyStackRef. Clang does a pretty good job of optimizing through it. GCC less so:https://gcc.godbolt.org/z/Ejs8c78qd.

That makes sense, thanks for the explanation. Since usingPyStackRef_IsExactly safely is sensitive to code generation changes, I might suggest using it only when we're sure it actually matters for performance, and default to using the variants that mask out the deferred bits everywhere by default since those are always safe. I'd guess that this wouldn't affect the performance improvement of this change much, since it should come from avoiding the tagging in_PyStackRef_FromPyObjectSteal. I don't feel super strongly though.