top: Remove MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE.

This feature originally provided a significant performance boost for Unix, but wasn't able to be enabled for MCU targets, and added significant extra complexity to generating .mpy files. The equivalent performance gain is now provided by MICROPY_OPT_LOAD_ATTR_FAST_PATH and MICROPY_OPT_MAP_LOOKUP_CACHE. See micropython#7680 for discussion. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
jimmo · Sep 13, 2021 · 93a2dd0 · 93a2dd0
1 parent 2d962e9
commit 93a2dd0
Show file tree

Hide file tree

Showing 32 changed files with 42 additions and 260 deletions.
diff --git a/docs/reference/mpyfiles.rst b/docs/reference/mpyfiles.rst
@@ -66,8 +66,6 @@ If importing an .mpy file fails then try the following:
     print('mpy flags:', end='')
     if arch:
         print(' -march=' + arch, end='')
-    if sys_mpy & 0x100:
-        print(' -mcache-lookup-bc', end='')
     if not sys_mpy & 0x200:
         print(' -mno-unicode', end='')
     print()

diff --git a/examples/embedding/mpconfigport_minimal.h b/examples/embedding/mpconfigport_minimal.h
@@ -45,7 +45,6 @@
 #define MICROPY_LONGINT_IMPL        (MICROPY_LONGINT_IMPL_NONE)
 #define MICROPY_STREAMS_NON_BLOCK   (0)
 #define MICROPY_OPT_COMPUTED_GOTO   (0)
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
 #define MICROPY_CAN_OVERRIDE_BUILTINS (0)
 #define MICROPY_BUILTIN_METHOD_CHECK_SELF_ARG (0)
 #define MICROPY_CPYTHON_COMPAT      (0)

diff --git a/mpy-cross/README.md b/mpy-cross/README.md
@@ -17,10 +17,7 @@ by the target MicroPython runtime (eg onto a pyboard's filesystem), and then
 imported like any other Python module using `import foo`.
 
 Different target runtimes may require a different format of the compiled
-bytecode, and such options can be passed to the cross compiler.  For example,
-the unix port of MicroPython requires the following:
-
-    $ ./mpy-cross -mcache-lookup-bc foo.py
+bytecode, and such options can be passed to the cross compiler.
 
 If the Python code contains `@native` or `@viper` annotations, then you must
 specify `-march` to match the target architecture.

diff --git a/mpy-cross/main.c b/mpy-cross/main.c
@@ -108,7 +108,6 @@ STATIC int usage(char **argv) {
         "Target specific options:\n"
         "-msmall-int-bits=number : set the maximum bits used to encode a small-int\n"
         "-mno-unicode : don't support unicode in compiled strings\n"
-        "-mcache-lookup-bc : cache map lookups in the bytecode\n"
         "-march=<arch> : set architecture for native emitter; x86, x64, armv6, armv7m, armv7em, armv7emsp, armv7emdp, xtensa, xtensawin\n"
         "\n"
         "Implementation specific options:\n", argv[0]
@@ -205,7 +204,6 @@ MP_NOINLINE int main_(int argc, char **argv) {
 
     // set default compiler configuration
     mp_dynamic_compiler.small_int_bits = 31;
-    mp_dynamic_compiler.opt_cache_map_lookup_in_bytecode = 0;
     mp_dynamic_compiler.py_builtins_str_unicode = 1;
     #if defined(__i386__)
     mp_dynamic_compiler.native_arch = MP_NATIVE_ARCH_X86;
@@ -264,10 +262,6 @@ MP_NOINLINE int main_(int argc, char **argv) {
                     return usage(argv);
                 }
                 // TODO check that small_int_bits is within range of host's capabilities
-            } else if (strcmp(argv[a], "-mno-cache-lookup-bc") == 0) {
-                mp_dynamic_compiler.opt_cache_map_lookup_in_bytecode = 0;
-            } else if (strcmp(argv[a], "-mcache-lookup-bc") == 0) {
-                mp_dynamic_compiler.opt_cache_map_lookup_in_bytecode = 1;
             } else if (strcmp(argv[a], "-mno-unicode") == 0) {
                 mp_dynamic_compiler.py_builtins_str_unicode = 0;
             } else if (strcmp(argv[a], "-municode") == 0) {

diff --git a/mpy-cross/mpconfigport.h b/mpy-cross/mpconfigport.h
@@ -57,8 +57,6 @@
 #define MICROPY_COMP_TRIPLE_TUPLE_ASSIGN (1)
 #define MICROPY_COMP_RETURN_IF_EXPR (1)
 
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
-
 #define MICROPY_READER_POSIX        (1)
 #define MICROPY_ENABLE_RUNTIME      (0)
 #define MICROPY_ENABLE_GC           (1)

diff --git a/ports/cc3200/mpconfigport.h b/ports/cc3200/mpconfigport.h
@@ -53,7 +53,6 @@
 #define MICROPY_LONGINT_IMPL                        (MICROPY_LONGINT_IMPL_MPZ)
 #define MICROPY_FLOAT_IMPL                          (MICROPY_FLOAT_IMPL_NONE)
 #define MICROPY_OPT_COMPUTED_GOTO                   (0)
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE    (0)
 #define MICROPY_READER_VFS                          (1)
 #ifndef DEBUG // we need ram on the launchxl while debugging
 #define MICROPY_CPYTHON_COMPAT                      (1)

diff --git a/ports/nrf/mpconfigport.h b/ports/nrf/mpconfigport.h
@@ -66,7 +66,6 @@
 #endif
 
 #define MICROPY_OPT_COMPUTED_GOTO   (0)
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
 #define MICROPY_OPT_MPZ_BITWISE     (0)
 
 // fatfs configuration used in ffconf.h

diff --git a/ports/stm32/mpconfigport.h b/ports/stm32/mpconfigport.h
@@ -59,7 +59,6 @@
 #ifndef MICROPY_OPT_COMPUTED_GOTO
 #define MICROPY_OPT_COMPUTED_GOTO   (1)
 #endif
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
 #ifndef MICROPY_OPT_LOAD_ATTR_FAST_PATH
 #define MICROPY_OPT_LOAD_ATTR_FAST_PATH (1)
 #endif

diff --git a/ports/unix/Makefile b/ports/unix/Makefile
@@ -269,7 +269,6 @@ ifneq ($(FROZEN_MANIFEST)$(FROZEN_MPY_DIR),)
 CFLAGS += -DMICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool
 CFLAGS += -DMICROPY_MODULE_FROZEN_MPY
 CFLAGS += -DMPZ_DIG_SIZE=16 # force 16 bits to work on both 32 and 64 bit archs
-MPY_CROSS_FLAGS += -mcache-lookup-bc
 endif
 
 ifneq ($(FROZEN_MANIFEST)$(FROZEN_DIR),)
@@ -285,9 +284,7 @@ endif
 CXXFLAGS += $(filter-out -Wmissing-prototypes -Wold-style-definition -std=gnu99,$(CFLAGS) $(CXXFLAGS_MOD))
 
 ifeq ($(MICROPY_FORCE_32BIT),1)
-RUN_TESTS_MPY_CROSS_FLAGS = --mpy-cross-flags='-mcache-lookup-bc -march=x86'
-else
-RUN_TESTS_MPY_CROSS_FLAGS = --mpy-cross-flags='-mcache-lookup-bc'
+RUN_TESTS_MPY_CROSS_FLAGS = --mpy-cross-flags='-march=x86'
 endif
 
 ifeq ($(CROSS_COMPILE),arm-linux-gnueabi-)

diff --git a/ports/unix/mpconfigport.h b/ports/unix/mpconfigport.h
@@ -79,9 +79,6 @@
 #endif
 #define MICROPY_STREAMS_POSIX_API   (1)
 #define MICROPY_OPT_COMPUTED_GOTO   (1)
-#ifndef MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (1)
-#endif
 #ifndef MICROPY_OPT_LOAD_ATTR_FAST_PATH
 #define MICROPY_OPT_LOAD_ATTR_FAST_PATH (1)
 #endif

diff --git a/ports/unix/variants/minimal/mpconfigvariant.h b/ports/unix/variants/minimal/mpconfigvariant.h
@@ -55,7 +55,6 @@
 #define MICROPY_LONGINT_IMPL        (MICROPY_LONGINT_IMPL_NONE)
 #define MICROPY_STREAMS_NON_BLOCK   (0)
 #define MICROPY_OPT_COMPUTED_GOTO   (0)
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
 #define MICROPY_OPT_LOAD_ATTR_FAST_PATH (0)
 #define MICROPY_OPT_MAP_LOOKUP_CACHE (0)
 #define MICROPY_CAN_OVERRIDE_BUILTINS (0)

diff --git a/ports/windows/Makefile b/ports/windows/Makefile
@@ -60,7 +60,6 @@ SRC_QSTR_AUTO_DEPS +=
 
 ifneq ($(FROZEN_MANIFEST),)
 CFLAGS += -DMICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool -DMICROPY_MODULE_FROZEN_MPY=1 -DMPZ_DIG_SIZE=16
-MPY_CROSS_FLAGS += -mcache-lookup-bc
 endif
 
 include $(TOP)/py/mkrules.mk
diff --git a/ports/windows/mpconfigport.h b/ports/windows/mpconfigport.h
@@ -59,7 +59,6 @@
 #define MICROPY_STREAMS_NON_BLOCK   (1)
 #define MICROPY_STREAMS_POSIX_API   (1)
 #define MICROPY_OPT_COMPUTED_GOTO   (0)
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (1)
 #define MICROPY_MODULE_WEAK_LINKS   (1)
 #define MICROPY_CAN_OVERRIDE_BUILTINS (1)
 #define MICROPY_VFS_POSIX_FILE      (1)

diff --git a/ports/windows/msvc/genhdr.targets b/ports/windows/msvc/genhdr.targets
@@ -123,7 +123,7 @@ using(var outFile = System.IO.File.CreateText(OutputFile)) {
         <PreprocessorDefinitions>MICROPY_MODULE_FROZEN_MPY=1;MICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       </ClCompile>
     </ItemGroup>
-    <Exec Command="$(PyPython) $(PyBaseDir)tools\makemanifest.py -v MPY_DIR=$(PyBaseDir) -v MPY_LIB_DIR=$(PyBaseDir)../micropython-lib -v PORT_DIR=$(PyWinDir) -f&quot;-mcache-lookup-bc&quot; -o $(PyBuildDir)frozen_content.c -b $(PyBuildDir) $(FrozenManifest)"/>
+    <Exec Command="$(PyPython) $(PyBaseDir)tools\makemanifest.py -v MPY_DIR=$(PyBaseDir) -v MPY_LIB_DIR=$(PyBaseDir)../micropython-lib -v PORT_DIR=$(PyWinDir) -o $(PyBuildDir)frozen_content.c -b $(PyBuildDir) $(FrozenManifest)"/>
     <WriteLinesToFile File="$(TLogLocation)frozen.read.1.tlog" Lines="$(FrozenManifest)" Overwrite="True"/>
   </Target>
 

diff --git a/py/bc.c b/py/bc.c
@@ -304,24 +304,10 @@ void mp_setup_code_state(mp_code_state_t *code_state, size_t n_args, size_t n_kw
 
 // The following table encodes the number of bytes that a specific opcode
 // takes up.  Some opcodes have an extra byte, defined by MP_BC_MASK_EXTRA_BYTE.
-// There are 4 special opcodes that have an extra byte only when
-// MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE is enabled (and they take a qstr):
-//     MP_BC_LOAD_NAME
-//     MP_BC_LOAD_GLOBAL
-//     MP_BC_LOAD_ATTR
-//     MP_BC_STORE_ATTR
 uint mp_opcode_format(const byte *ip, size_t *opcode_size, bool count_var_uint) {
     uint f = MP_BC_FORMAT(*ip);
     const byte *ip_start = ip;
     if (f == MP_BC_FORMAT_QSTR) {
-        if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC) {
-            if (*ip == MP_BC_LOAD_NAME
-                || *ip == MP_BC_LOAD_GLOBAL
-                || *ip == MP_BC_LOAD_ATTR
-                || *ip == MP_BC_STORE_ATTR) {
-                ip += 1;
-            }
-        }
         ip += 3;
     } else {
         int extra_byte = (*ip & MP_BC_MASK_EXTRA_BYTE) == 0;

diff --git a/py/dynruntime.mk b/py/dynruntime.mk
@@ -46,15 +46,13 @@ ifeq ($(ARCH),x86)
 # x86
 CROSS =
 CFLAGS += -m32 -fno-stack-protector
-MPY_CROSS_FLAGS += -mcache-lookup-bc
 MICROPY_FLOAT_IMPL ?= double
 
 else ifeq ($(ARCH),x64)
 
 # x64
 CROSS =
 CFLAGS += -fno-stack-protector
-MPY_CROSS_FLAGS += -mcache-lookup-bc
 MICROPY_FLOAT_IMPL ?= double
 
 else ifeq ($(ARCH),armv7m)

diff --git a/py/emitbc.c b/py/emitbc.c
@@ -560,9 +560,6 @@ void mp_emit_bc_load_global(emit_t *emit, qstr qst, int kind) {
     MP_STATIC_ASSERT(MP_BC_LOAD_NAME + MP_EMIT_IDOP_GLOBAL_GLOBAL == MP_BC_LOAD_GLOBAL);
     (void)qst;
     emit_write_bytecode_byte_qstr(emit, 1, MP_BC_LOAD_NAME + kind, qst);
-    if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC) {
-        emit_write_bytecode_raw_byte(emit, 0);
-    }
 }
 
 void mp_emit_bc_load_method(emit_t *emit, qstr qst, bool is_super) {
@@ -596,9 +593,6 @@ void mp_emit_bc_attr(emit_t *emit, qstr qst, int kind) {
         }
         emit_write_bytecode_byte_qstr(emit, -2, MP_BC_STORE_ATTR, qst);
     }
-    if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC) {
-        emit_write_bytecode_raw_byte(emit, 0);
-    }
 }
 
 void mp_emit_bc_store_local(emit_t *emit, qstr qst, mp_uint_t local_num, int kind) {

diff --git a/py/mpconfig.h b/py/mpconfig.h
@@ -394,10 +394,8 @@
 
 // Configure dynamic compiler macros
 #if MICROPY_DYNAMIC_COMPILER
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC (mp_dynamic_compiler.opt_cache_map_lookup_in_bytecode)
 #define MICROPY_PY_BUILTINS_STR_UNICODE_DYNAMIC (mp_dynamic_compiler.py_builtins_str_unicode)
 #else
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE
 #define MICROPY_PY_BUILTINS_STR_UNICODE_DYNAMIC MICROPY_PY_BUILTINS_STR_UNICODE
 #endif
 
@@ -491,13 +489,6 @@
 #define MICROPY_OPT_COMPUTED_GOTO (0)
 #endif
 
-// Whether to cache result of map lookups in LOAD_NAME, LOAD_GLOBAL, LOAD_ATTR,
-// STORE_ATTR bytecodes.  Uses 1 byte extra RAM for each of these opcodes and
-// uses a bit of extra code ROM, but greatly improves lookup speed.
-#ifndef MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE
-#define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (0)
-#endif
-
 // Optimise the fast path for loading attributes from instance types. Increases
 // Thumb2 code size by about 48 bytes.
 #ifndef MICROPY_OPT_LOAD_ATTR_FAST_PATH

diff --git a/py/mpstate.h b/py/mpstate.h
@@ -44,7 +44,6 @@
 #if MICROPY_DYNAMIC_COMPILER
 typedef struct mp_dynamic_compiler_t {
     uint8_t small_int_bits; // must be <= host small_int_bits
-    bool opt_cache_map_lookup_in_bytecode;
     bool py_builtins_str_unicode;
     uint8_t native_arch;
     uint8_t nlr_buf_num_regs;

diff --git a/py/persistentcode.h b/py/persistentcode.h
@@ -41,16 +41,15 @@
 #define MPY_FEATURE_ENCODE_ARCH(arch) ((arch) << 2)
 #define MPY_FEATURE_DECODE_ARCH(feat) ((feat) >> 2)
 
-// The feature flag bits encode the compile-time config options that
-// affect the generate bytecode.
+// The feature flag bits encode the compile-time config options that affect
+// the generate bytecode. Note: position 0 is now unused
+// (formerly MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE).
 #define MPY_FEATURE_FLAGS ( \
-    ((MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) << 0) \
-    | ((MICROPY_PY_BUILTINS_STR_UNICODE) << 1) \
+    ((MICROPY_PY_BUILTINS_STR_UNICODE) << 1) \
     )
 // This is a version of the flags that can be configured at runtime.
 #define MPY_FEATURE_FLAGS_DYNAMIC ( \
-    ((MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC) << 0) \
-    | ((MICROPY_PY_BUILTINS_STR_UNICODE_DYNAMIC) << 1) \
+    ((MICROPY_PY_BUILTINS_STR_UNICODE_DYNAMIC) << 1) \
     )
 
 // Define the host architecture

diff --git a/py/profile.c b/py/profile.c
@@ -540,29 +540,20 @@ STATIC const byte *mp_prof_opcode_decode(const byte *ip, const mp_uint_t *const_
             instruction->qstr_opname = MP_QSTR_LOAD_NAME;
             instruction->arg = qst;
             instruction->argobj = MP_OBJ_NEW_QSTR(qst);
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                instruction->argobjex_cache = MP_OBJ_NEW_SMALL_INT(*ip++);
-            }
             break;
 
         case MP_BC_LOAD_GLOBAL:
             DECODE_QSTR;
             instruction->qstr_opname = MP_QSTR_LOAD_GLOBAL;
             instruction->arg = qst;
             instruction->argobj = MP_OBJ_NEW_QSTR(qst);
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                instruction->argobjex_cache = MP_OBJ_NEW_SMALL_INT(*ip++);
-            }
             break;
 
         case MP_BC_LOAD_ATTR:
             DECODE_QSTR;
             instruction->qstr_opname = MP_QSTR_LOAD_ATTR;
             instruction->arg = qst;
             instruction->argobj = MP_OBJ_NEW_QSTR(qst);
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                instruction->argobjex_cache = MP_OBJ_NEW_SMALL_INT(*ip++);
-            }
             break;
 
         case MP_BC_LOAD_METHOD:
@@ -618,9 +609,6 @@ STATIC const byte *mp_prof_opcode_decode(const byte *ip, const mp_uint_t *const_
             instruction->qstr_opname = MP_QSTR_STORE_ATTR;
             instruction->arg = qst;
             instruction->argobj = MP_OBJ_NEW_QSTR(qst);
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                instruction->argobjex_cache = MP_OBJ_NEW_SMALL_INT(*ip++);
-            }
             break;
 
         case MP_BC_STORE_SUBSCR:

diff --git a/py/showbc.c b/py/showbc.c
@@ -208,25 +208,16 @@ const byte *mp_bytecode_print_str(const mp_print_t *print, const byte *ip) {
         case MP_BC_LOAD_NAME:
             DECODE_QSTR;
             mp_printf(print, "LOAD_NAME %s", qstr_str(qst));
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                mp_printf(print, " (cache=%u)", *ip++);
-            }
             break;
 
         case MP_BC_LOAD_GLOBAL:
             DECODE_QSTR;
             mp_printf(print, "LOAD_GLOBAL %s", qstr_str(qst));
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                mp_printf(print, " (cache=%u)", *ip++);
-            }
             break;
 
         case MP_BC_LOAD_ATTR:
             DECODE_QSTR;
             mp_printf(print, "LOAD_ATTR %s", qstr_str(qst));
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                mp_printf(print, " (cache=%u)", *ip++);
-            }
             break;
 
         case MP_BC_LOAD_METHOD:
@@ -270,9 +261,6 @@ const byte *mp_bytecode_print_str(const mp_print_t *print, const byte *ip) {
         case MP_BC_STORE_ATTR:
             DECODE_QSTR;
             mp_printf(print, "STORE_ATTR %s", qstr_str(qst));
-            if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE) {
-                mp_printf(print, " (cache=%u)", *ip++);
-            }
             break;
 
         case MP_BC_STORE_SUBSCR: