You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Changes to specialized instructions requires a magic number change. #109256
Until #107971 only changes to the set of instructions present in the pyc file required a new version change.
Now, removing or adding any opcode requires a pyc file.
There are a few problems with this.
pyc files have to be regenerated for changes unrelated to the on disk format.
This makes it much harder to add or remove specializations during the beta phase, as it will invalidate pyc files
It consumes an unnecessary number of magic numbers. This is not a big deal, but want to keep to a limit of 50 per release
It reduces stability in layout and thus performance, making meaningful benchmarking harder.
Allocate "external" opcodes to odd numbers.
Allocate instrumented opcodes to the top region (as we do now)
Allocate specialized and other internal opcodes to even numbers
It would also be nice to be able to specify the most common opcodes, so that tooling does a decent job of laying out the tables.
We might also consider generating a hash of the opcodes and checking it test_opcodes to ensure that the magic number is updated when necessary.
If we keep HAVE_ARGUMENT working, then we have the constraint that all ops with arg are below it and all ops without are above (except the instrumented). So it means we defined 4 ranges + one range for the instrumented instructions.
Internal instructions don't exist as far as pyc files or dis are concerned, so I don't think HAVE_ARGUMENT is relevant.
They already went all over the place before #107971 as we just filled in the gaps with them.
Do we support HAVE_ARGUMENT any more?
Maybe we could define it as 0 and say that all instruction have an operand, it is just that some ignore their operand?
Internal instructions don't exist as far as pyc files or dis are concerned, so I don't think HAVE_ARGUMENT is relevant. They already went all over the place before #107971 as we just filled in the gaps with them.
Cool.
Do we support HAVE_ARGUMENT any more? Maybe we could define it as 0 and say that all instruction have an operand, it is just that some ignore their operand?
We do, for backward compatibility, but we also have dis.has_arg which is better.
Activity
iritkatriel commentedon Sep 11, 2023
What distinguishes external/internal opcodes in the DSL?
odd/even assumes at most half of each type. We could allocate ranges for them so it's more flexible.
markshannon commentedon Sep 11, 2023
opcode._specializationshas the information. It must have got it from somewhereSure. It might be better as well as we have 117 normal instructions and 70 specializations, so it isn't a 50/50 split.
iritkatriel commentedon Sep 11, 2023
If we keep HAVE_ARGUMENT working, then we have the constraint that all ops with arg are below it and all ops without are above (except the instrumented). So it means we defined 4 ranges + one range for the instrumented instructions.
iritkatriel commentedon Sep 11, 2023
Unless we say that internal instructions don't respect this, and they go in the end before the instrumented ones.
markshannon commentedon Sep 11, 2023
Internal instructions don't exist as far as pyc files or
disare concerned, so I don't thinkHAVE_ARGUMENTis relevant.They already went all over the place before #107971 as we just filled in the gaps with them.
Do we support
HAVE_ARGUMENTany more?Maybe we could define it as 0 and say that all instruction have an operand, it is just that some ignore their operand?
markshannon commentedon Sep 11, 2023
If that's too radical, then deprecate
HAVE_ARGUMENT, and we'll work around it for a couple more releases.iritkatriel commentedon Sep 11, 2023
Cool.
We do, for backward compatibility, but we also have dis.has_arg which is better.
gh-109256: allocate opcode IDs for internal opcodes in their own range (
pythongh-109256: allocate opcode IDs for internal opcodes in their ow…
hugovk commentedon Nov 9, 2023
Closing as the PR has been merged, please re-open if needed. Thanks!