gh-131798: JIT: Propagate the result in _BINARY_OP_SUBSCR_TUPLE_INT
#133003
Conversation
Python/optimizer_bytecodes.c
Outdated
| assert(index >= 0); | ||
| assert(index < sym_tuple_length(left)); |
There was a problem hiding this comment.
I'm not sure these asserts are actually needed. The instruction will deopt when this is not true so perhaps we can remove them?
There was a problem hiding this comment.
Interesting, we are actually hitting the assertion:
Python/optimizer_cases.c.h:630: optimize_uops: Assertion `index < sym_tuple_length(left)' failed.I guess I need to move the DEOPT_IF checks into a separate guard for it to work?
There was a problem hiding this comment.
Check out the code for sym_tuple_length. It can return -1 if the length is not known. So for example if you propagate a PyTuple_Type, but not the length, it will fail.
You need to check that the length is not -1.
There was a problem hiding this comment.
Got it, thanks! I somehow didn't realize you can know something is a tuple but not know its length..
Python/optimizer_bytecodes.c
Outdated
| long index = PyLong_AsLong(sym_get_const(ctx, right)); | ||
| assert(index >= 0); | ||
| assert(index < sym_tuple_length(left)); | ||
| res = sym_tuple_getitem(ctx, left, index); |
There was a problem hiding this comment.
This is a bit unfortunate. I would wish we could automatically evaluate this, but it seems tuples are a special category so we can't.
There was a problem hiding this comment.
I haven't fully digested your PR yet, but maybe there is some way to extend it to support tuples as well?
|
Thanks! |
More context: #132851 (comment)
This propagates the information about tuple elements after
_BINARY_OP_SUBSCR_TUPLE_INTwhen the RHS is a constant.For example in
we can now deduce that
xis1.