Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort bytecode opcodes into groups based on their format #5099

Open
wants to merge 5 commits into
base: master
from

Conversation

@dpgeorge
Copy link
Member

commented Sep 12, 2019

This PR is a set of related commits to reorganise the VM opcodes to simplify their encoding/decoding. Up until now opcodes have just been "randomly" assigned, and there's a large table in py/bc.c and tools/mpy-tool.py which maps opcode to its format.

This PR makes the following changes:

  • sort opcodes into groups depending on their arguments, eg all single-byte opcodes together, all ones that take a qstr together, etc; this way certain bits in the opcode can be used to tell what the argument is and whether it has an extra byte
  • update tests to take into account changes in opcodes
  • remove the big opcode format tables (in py/bc.c and tools/mpy-tool.py) and replace them with simple macros/formulas based on the new opcode numbering
  • replace some magic numbers with macro constants

The main aim with these changes is to improve readability of the opcode encoding as well as future maintainability (removing the big format table is the main thing, because that has had some bugs in the past).

The only real disadvantage of this change is that 3rd-party tools that rely on the opcodes being what they are will need to be updated (and maybe even simplified due to opcodes now having meaning). Also the MPY version needs to be updated.

Code size changes with this PR are:

   bare-arm:    -4 -0.006% 
minimal x86:   -32 -0.021% 
   unix x64:   -96 -0.019% 
unix nanbox:  -132 -0.030% 
      stm32:   -88 -0.024% PYBV10
     cc3200:   -80 -0.043% 
    esp8266:  -124 -0.019% 
      esp32:   -80 -0.007% GENERIC [incl -64(data)]
        nrf:   -24 -0.016% pca10040
       samd:   -16 -0.016% ADAFRUIT_ITSYBITSY_M4_EXPRESS
``
@stinos

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

The main aim with these changes is to improve readability of the opcode encoding as well as future maintainability (removing the big format table is the main thing, because that has had some bugs in the past).

+1, this definitely makes things more clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.