GH-144179: Use recorded values to make optimizer more robust #144437
Conversation
* Add three new symbol kinds * Do not smuggle code object in _PUSH_FRAME operand * Fix small bug in predicate analysis
markshannon
requested review from
Fidget-Spinner,
savannahostrowski and
tomasr8
as code owners
Fidget-Spinner
left a comment
Fidget-Spinner
left a comment
There was a problem hiding this comment.
You need tests for the new symbolic types in the optimizer_symbols.c as well.
| tier2 op(_GUARD_CODE, (version/2 -- )) { | ||
| PyObject *code = PyStackRef_AsPyObjectBorrow(frame->f_executable); | ||
| EXIT_IF(code == Py_None); | ||
| EXIT_IF(((PyCodeObject *)code)->co_version != version); |
There was a problem hiding this comment.
Why do we need this? Doesn't a _CHECK_VALIIDTY suffice?
There was a problem hiding this comment.
_CHECK_VALIDITY only checks if the executor is valid, not that we are executing the expected path.
We need to check that we have the expected code object.
There was a problem hiding this comment.
I don't remember: do we have code version watchers? if so they should work with _CHECK_VALIDITY right?
There was a problem hiding this comment.
Watchers only work if they've been wired up, which is the job of the optimizer, not the front-end.
| sym->tag = JIT_SYM_RECORDED_VALUE_TAG; | ||
| sym->recorded_value.known_type = true; | ||
| sym->recorded_value.value = value; | ||
| } |
There was a problem hiding this comment.
Why do we not set bottom here and for the one below?
There was a problem hiding this comment.
I presume you mean when sym->cls.type != Py_TYPE(value).
I'll add that.
…ythonGH-144437) * Add three new symbol kinds * Do not smuggle code object in _PUSH_FRAME operand * Fix small bug in predicate analysis
2 participants
Uh oh!
There was an error while loading. Please reload this page.