Skip to content

py/misc: use __builtin_strcmp to enable compile-time optimization #17196

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

romanz
Copy link

@romanz romanz commented Apr 27, 2025

(cherry picked from commit e2bba3c)

Summary

Otherwise, it seems that the compiler doesn't optimize the strcmp generated here:
https://github.com/dpgeorge/micropython/blob/154b4eb354d97c7c28253bdc5212b2e58ea6462e/py/misc.h#L286

This issue was reported in #7659 (comment) and encountered when enabling MICROPY_ROM_TEXT_COMPRESSION in trezor/trezor-firmware#4961.

Following this discussion, I have tried to replace strcmp by __builtin_strcmp and it seems to work as intended.

Testing

Tested on a simple function:

mp_obj_t mp_foo_bar() {
    mp_rom_error_text_t err = MP_ERROR_TEXT("exceptions must derive from BaseException");
    return mp_obj_new_exception_msg(&mp_type_TypeError, err);
}

using

arm-none-eabi-objdump --visualize-jumps=color --disassembler-color=on -S --disassemble=mp_foo_bar core/build/firmware/firmware.elf
After the fix
mp_obj_t mp_foo_bar() {
 c0a2b14:           b507        push    {r0, r1, r2, lr}
 c0a2b16:           4b0a        ldr     r3, [pc, #40]   @ (c0a2b40 <mp_foo_bar+0x2c>)
 c0a2b18:           681b        ldr     r3, [r3, #0]
 c0a2b1a:           9301        str     r3, [sp, #4]
 c0a2b1c:           f04f 0300   mov.w   r3, #0
    mp_rom_error_text_t err = MP_ERROR_TEXT("exceptions must derive from BaseException");
    return mp_obj_new_exception_msg(&mp_type_TypeError, err);
 c0a2b20:           4b07        ldr     r3, [pc, #28]   @ (c0a2b40 <mp_foo_bar+0x2c>)
 c0a2b22:           681a        ldr     r2, [r3, #0]
 c0a2b24:           9b01        ldr     r3, [sp, #4]
 c0a2b26:           405a        eors    r2, r3
 c0a2b28:           f04f 0300   mov.w   r3, #0
 c0a2b2c:       /-- d001        beq.n   c0a2b32 <mp_foo_bar+0x1e>
 c0a2b2e:       |   f009 fb87   bl      c0ac240 <__stack_chk_fail>
 c0a2b32:       \-> 4904        ldr     r1, [pc, #16]   @ (c0a2b44 <mp_foo_bar+0x30>)
 c0a2b34:           4804        ldr     r0, [pc, #16]   @ (c0a2b48 <mp_foo_bar+0x34>)
}
 c0a2b36:           b003        add     sp, #12
 c0a2b38:           f85d eb04   ldr.w   lr, [sp], #4
    return mp_obj_new_exception_msg(&mp_type_TypeError, err);
 c0a2b3c:           f025 be9e   b.w     c0c887c <mp_obj_new_exception_msg>
 c0a2b40:           30007648    .word   0x30007648
 c0a2b44:           0c10d859    .word   0x0c10d859
 c0a2b48:           0c13b520    .word   0x0c13b520
Before the fix
mp_obj_t mp_foo_bar() {
 c0bae90:                                                                    b507       push    {r0, r1, r2, lr}
 c0bae92:                                                                    4b56       ldr     r3, [pc, #344]  @ (c0bafec <mp_foo_bar+0x15c>)
#define MP_MAX_UNCOMPRESSED_TEXT_LEN (45)
MP_COMPRESSED_DATA("objec\364'%s\247isn'\364'%q\247unsupporte\344allocatio\356attribut\345typ\345failed\254ha\363memor\371unpac\353value\363can'\364t\357nam\345%q\272fo\362n\357%\344%d\251%\361%\363%\365'%s'\254(expecte\344BaseExceptio\356StopIteratio\356allocatin\347a\356argu>
MP_MATCH_COMPRESSED("name '%q' isn't defined", "\377\217\203\202\244")
 c0bae94:                                                                    4956       ldr     r1, [pc, #344]  @ (c0baff0 <mp_foo_bar+0x160>)
 c0bae96:                                                                    4857       ldr     r0, [pc, #348]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0bae98:                                                                    681b       ldr     r3, [r3, #0]
 c0bae9a:                                                                    9301       str     r3, [sp, #4]
 c0bae9c:                                                                    f04f 0300  mov.w   r3, #0
 c0baea0:                                                                    f075 fa81  bl      c1303a6 <strcmp>
 c0baea4:                                                                    2800       cmp     r0, #0
 c0baea6:       /----------------------------------------------------------- d077       beq.n   c0baf98 <mp_foo_bar+0x108>
MP_MATCH_COMPRESSED("can't convert %s to int", "\377\215\242\226\216\254")
 c0baea8:       |                                                            4953       ldr     r1, [pc, #332]  @ (c0baff8 <mp_foo_bar+0x168>)
 c0baeaa:       |                                                            4852       ldr     r0, [pc, #328]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baeac:       |                                                            f075 fa7b  bl      c1303a6 <strcmp>
 c0baeb0:       |                                                            2800       cmp     r0, #0
 c0baeb2:       |  /-------------------------------------------------------- d073       beq.n   c0baf9c <mp_foo_bar+0x10c>
MP_MATCH_COMPRESSED("unsupported type for %q: '%s'", "\377\204\207\221\220\201")
 c0baeb4:       |  |                                                         4951       ldr     r1, [pc, #324]  @ (c0baffc <mp_foo_bar+0x16c>)
 c0baeb6:       |  |                                                         484f       ldr     r0, [pc, #316]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baeb8:       |  |                                                         f075 fa75  bl      c1303a6 <strcmp>
 c0baebc:       |  |                                                         2800       cmp     r0, #0
 c0baebe:       |  |  /----------------------------------------------------- d06f       beq.n   c0bafa0 <mp_foo_bar+0x110>
MP_MATCH_COMPRESSED("negative shift count", "\377\265\267\243")
 c0baec0:       |  |  |                                                      494f       ldr     r1, [pc, #316]  @ (c0bb000 <mp_foo_bar+0x170>)
 c0baec2:       |  |  |                                                      484c       ldr     r0, [pc, #304]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baec4:       |  |  |                                                      f075 fa6f  bl      c1303a6 <strcmp>
 c0baec8:       |  |  |                                                      2800       cmp     r0, #0
 c0baeca:       |  |  |  /-------------------------------------------------- d06b       beq.n   c0bafa4 <mp_foo_bar+0x114>
MP_MATCH_COMPRESSED("unsupported types for %q: '%s', '%s'", "\377\204\272\221\220\230\201")
 c0baecc:       |  |  |  |                                                   494d       ldr     r1, [pc, #308]  @ (c0bb004 <mp_foo_bar+0x174>)
 c0baece:       |  |  |  |                                                   4849       ldr     r0, [pc, #292]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baed0:       |  |  |  |                                                   f075 fa69  bl      c1303a6 <strcmp>
 c0baed4:       |  |  |  |                                                   2800       cmp     r0, #0
 c0baed6:       |  |  |  |  /----------------------------------------------- d067       beq.n   c0bafa8 <mp_foo_bar+0x118>
MP_MATCH_COMPRESSED("divide by zero", "\377\246\237\274")
 c0baed8:       |  |  |  |  |                                                494b       ldr     r1, [pc, #300]  @ (c0bb008 <mp_foo_bar+0x178>)
 c0baeda:       |  |  |  |  |                                                4846       ldr     r0, [pc, #280]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baedc:       |  |  |  |  |                                                f075 fa63  bl      c1303a6 <strcmp>
 c0baee0:       |  |  |  |  |                                                2800       cmp     r0, #0
 c0baee2:       |  |  |  |  |  /-------------------------------------------- d063       beq.n   c0bafac <mp_foo_bar+0x11c>
MP_MATCH_COMPRESSED("'%s' object isn't callable", "\377\201\200\202\241")

 c0baee4:       |  |  |  |  |  |                                             4949       ldr     r1, [pc, #292]  @ (c0bb00c <mp_foo_bar+0x17c>)
 c0baee6:       |  |  |  |  |  |                                             4843       ldr     r0, [pc, #268]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baee8:       |  |  |  |  |  |                                             f075 fa5d  bl      c1303a6 <strcmp>
 c0baeec:       |  |  |  |  |  |                                             2800       cmp     r0, #0
 c0baeee:       |  |  |  |  |  |  /----------------------------------------- d05f       beq.n   c0bafb0 <mp_foo_bar+0x120>
MP_MATCH_COMPRESSED("need more than %d values to unpack", "\377\264\262\270\223\214\216\213")
 c0baef0:       |  |  |  |  |  |  |                                          4947       ldr     r1, [pc, #284]  @ (c0bb010 <mp_foo_bar+0x180>)
 c0baef2:       |  |  |  |  |  |  |                                          4840       ldr     r0, [pc, #256]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baef4:       |  |  |  |  |  |  |                                          f075 fa57  bl      c1303a6 <strcmp>
 c0baef8:       |  |  |  |  |  |  |                                          2800       cmp     r0, #0
 c0baefa:       |  |  |  |  |  |  |  /-------------------------------------- d05b       beq.n   c0bafb4 <mp_foo_bar+0x124>
MP_MATCH_COMPRESSED("too many values to unpack (expected %d)", "\377\271\261\214\216\213\231\224")
 c0baefc:       |  |  |  |  |  |  |  |                                       4945       ldr     r1, [pc, #276]  @ (c0bb014 <mp_foo_bar+0x184>)
 c0baefe:       |  |  |  |  |  |  |  |                                       483d       ldr     r0, [pc, #244]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf00:       |  |  |  |  |  |  |  |                                       f075 fa51  bl      c1303a6 <strcmp>
 c0baf04:       |  |  |  |  |  |  |  |                                       2800       cmp     r0, #0
 c0baf06:       |  |  |  |  |  |  |  |  /----------------------------------- d057       beq.n   c0bafb8 <mp_foo_bar+0x128>
MP_MATCH_COMPRESSED("argument has wrong type", "\377\236\211\273\207")
 c0baf08:       |  |  |  |  |  |  |  |  |                                    4943       ldr     r1, [pc, #268]  @ (c0bb018 <mp_foo_bar+0x188>)
 c0baf0a:       |  |  |  |  |  |  |  |  |                                    483a       ldr     r0, [pc, #232]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf0c:       |  |  |  |  |  |  |  |  |                                    f075 fa4b  bl      c1303a6 <strcmp>
 c0baf10:       |  |  |  |  |  |  |  |  |                                    2800       cmp     r0, #0
 c0baf12:       |  |  |  |  |  |  |  |  |  /-------------------------------- d053       beq.n   c0bafbc <mp_foo_bar+0x12c>
MP_MATCH_COMPRESSED("type object '%q' has no attribute '%q'", "\377\207\200\203\211\222\206\203")
 c0baf14:       |  |  |  |  |  |  |  |  |  |                                 4941       ldr     r1, [pc, #260]  @ (c0bb01c <mp_foo_bar+0x18c>)
 c0baf16:       |  |  |  |  |  |  |  |  |  |                                 4837       ldr     r0, [pc, #220]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf18:       |  |  |  |  |  |  |  |  |  |                                 f075 fa45  bl      c1303a6 <strcmp>
 c0baf1c:       |  |  |  |  |  |  |  |  |  |                                 2800       cmp     r0, #0
 c0baf1e:       |  |  |  |  |  |  |  |  |  |  /----------------------------- d04f       beq.n   c0bafc0 <mp_foo_bar+0x130>
MP_MATCH_COMPRESSED("'%s' object has no attribute '%q'", "\377\201\200\211\222\206\203")
 c0baf20:       |  |  |  |  |  |  |  |  |  |  |                              493f       ldr     r1, [pc, #252]  @ (c0bb020 <mp_foo_bar+0x190>)
 c0baf22:       |  |  |  |  |  |  |  |  |  |  |                              4834       ldr     r0, [pc, #208]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf24:       |  |  |  |  |  |  |  |  |  |  |                              f075 fa3f  bl      c1303a6 <strcmp>
 c0baf28:       |  |  |  |  |  |  |  |  |  |  |                              2800       cmp     r0, #0
 c0baf2a:       |  |  |  |  |  |  |  |  |  |  |  /-------------------------- d04b       beq.n   c0bafc4 <mp_foo_bar+0x134>
MP_MATCH_COMPRESSED("'%s' object isn't iterable", "\377\201\200\202\256")
 c0baf2c:       |  |  |  |  |  |  |  |  |  |  |  |                           493d       ldr     r1, [pc, #244]  @ (c0bb024 <mp_foo_bar+0x194>)
 c0baf2e:       |  |  |  |  |  |  |  |  |  |  |  |                           4831       ldr     r0, [pc, #196]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf30:       |  |  |  |  |  |  |  |  |  |  |  |                           f075 fa39  bl      c1303a6 <strcmp>
 c0baf34:       |  |  |  |  |  |  |  |  |  |  |  |                           2800       cmp     r0, #0
 c0baf36:       |  |  |  |  |  |  |  |  |  |  |  |  /----------------------- d047       beq.n   c0bafc8 <mp_foo_bar+0x138>
MP_MATCH_COMPRESSED("'%s' object isn't an iterator", "\377\201\200\202\235\257")
 c0baf38:       |  |  |  |  |  |  |  |  |  |  |  |  |                        493b       ldr     r1, [pc, #236]  @ (c0bb028 <mp_foo_bar+0x198>)
 c0baf3a:       |  |  |  |  |  |  |  |  |  |  |  |  |                        482e       ldr     r0, [pc, #184]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf3c:       |  |  |  |  |  |  |  |  |  |  |  |  |                        f075 fa33  bl      c1303a6 <strcmp>
 c0baf40:       |  |  |  |  |  |  |  |  |  |  |  |  |                        2800       cmp     r0, #0
 c0baf42:       |  |  |  |  |  |  |  |  |  |  |  |  |  /-------------------- d043       beq.n   c0bafcc <mp_foo_bar+0x13c>
MP_MATCH_COMPRESSED("generator raised StopIteration", "\377\251\266\233")
 c0baf44:       |  |  |  |  |  |  |  |  |  |  |  |  |  |                     4939       ldr     r1, [pc, #228]  @ (c0bb02c <mp_foo_bar+0x19c>)
 c0baf46:       |  |  |  |  |  |  |  |  |  |  |  |  |  |                     482b       ldr     r0, [pc, #172]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf48:       |  |  |  |  |  |  |  |  |  |  |  |  |  |                     f075 fa2d  bl      c1303a6 <strcmp>
 c0baf4c:       |  |  |  |  |  |  |  |  |  |  |  |  |  |                     2800       cmp     r0, #0
 c0baf4e:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  /----------------- d03f       beq.n   c0bafd0 <mp_foo_bar+0x140>
MP_MATCH_COMPRESSED("exceptions must derive from BaseException", "\377\247\263\245\250\232")
 c0baf50:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |                  4928       ldr     r1, [pc, #160]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf52:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |                  4608       mov     r0, r1
 c0baf54:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |                  f075 fa27  bl      c1303a6 <strcmp>

 c0baf58:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |                  2800       cmp     r0, #0
 c0baf5a:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  /-------------- d03b       beq.n   c0bafd4 <mp_foo_bar+0x144>
MP_MATCH_COMPRESSED("can't import name %q", "\377\215\253\217\225")
 c0baf5c:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |               4934       ldr     r1, [pc, #208]  @ (c0bb030 <mp_foo_bar+0x1a0>)
 c0baf5e:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |               4825       ldr     r0, [pc, #148]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf60:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |               f075 fa21  bl      c1303a6 <strcmp>
 c0baf64:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |               2800       cmp     r0, #0
 c0baf66:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  /----------- d037       beq.n   c0bafd8 <mp_foo_bar+0x148>
MP_MATCH_COMPRESSED("memory allocation failed, heap is locked", "\377\212\205\210\252\255\260")
 c0baf68:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |            4932       ldr     r1, [pc, #200]  @ (c0bb034 <mp_foo_bar+0x1a4>)
 c0baf6a:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |            4822       ldr     r0, [pc, #136]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf6c:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |            f075 fa1b  bl      c1303a6 <strcmp>
 c0baf70:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |            2800       cmp     r0, #0
 c0baf72:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  /-------- d033       beq.n   c0bafdc <mp_foo_bar+0x14c>
MP_MATCH_COMPRESSED("memory allocation failed, allocating %u bytes", "\377\212\205\210\234\227\240")
 c0baf74:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         4930       ldr     r1, [pc, #192]  @ (c0bb038 <mp_foo_bar+0x1a8>)
 c0baf76:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         481f       ldr     r0, [pc, #124]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf78:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         f075 fa15  bl      c1303a6 <strcmp>
 c0baf7c:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         491d       ldr     r1, [pc, #116]  @ (c0baff4 <mp_foo_bar+0x164>)
 c0baf7e:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         4b2f       ldr     r3, [pc, #188]  @ (c0bb03c <mp_foo_bar+0x1ac>)
 c0baf80:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         2800       cmp     r0, #0
 c0baf82:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         bf08       it      eq
 c0baf84:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |         4619       moveq   r1, r3
    return mp_obj_new_exception_msg(&mp_type_TypeError, MP_ERROR_TEXT("exceptions must derive from BaseException"));
 c0baf86:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  /----> 4b19       ldr     r3, [pc, #100]  @ (c0bafec <mp_foo_bar+0x15c>)
 c0baf88:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |      681a       ldr     r2, [r3, #0]
 c0baf8a:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |      9b01       ldr     r3, [sp, #4]
 c0baf8c:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |      405a       eors    r2, r3
 c0baf8e:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |      f04f 0300  mov.w   r3, #0
 c0baf92:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  /-- d025       beq.n   c0bafe0 <mp_foo_bar+0x150>
 c0baf94:       |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |   f00d ff92  bl      c0c8ebc <__stack_chk_fail>
MP_MATCH_COMPRESSED("name '%q' isn't defined", "\377\217\203\202\244")
 c0baf98:       \--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb040 <mp_foo_bar+0x1b0>)
 c0baf9a:          |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7f4       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("can't convert %s to int", "\377\215\242\226\216\254")
 c0baf9c:          \--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb044 <mp_foo_bar+0x1b4>)
 c0baf9e:             |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7f2       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("unsupported type for %q: '%s'", "\377\204\207\221\220\201")
 c0bafa0:             \--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb048 <mp_foo_bar+0x1b8>)
 c0bafa2:                |  |  |  |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7f0       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("negative shift count", "\377\265\267\243")
 c0bafa4:                \--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb04c <mp_foo_bar+0x1bc>)
 c0bafa6:                   |  |  |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7ee       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("unsupported types for %q: '%s', '%s'", "\377\204\272\221\220\230\201")
 c0bafa8:                   \--|--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb050 <mp_foo_bar+0x1c0>)
 c0bafaa:                      |  |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7ec       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("divide by zero", "\377\246\237\274")
 c0bafac:                      \--|--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb054 <mp_foo_bar+0x1c4>)
 c0bafae:                         |  |  |  |  |  |  |  |  |  |  |  |  +--|-- e7ea       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("'%s' object isn't callable", "\377\201\200\202\241")
 c0bafb0:                         \--|--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb058 <mp_foo_bar+0x1c8>)
 c0bafb2:                            |  |  |  |  |  |  |  |  |  |  |  +--|-- e7e8       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("need more than %d values to unpack", "\377\264\262\270\223\214\216\213")
 c0bafb4:                            \--|--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb05c <mp_foo_bar+0x1cc>)
 c0bafb6:                               |  |  |  |  |  |  |  |  |  |  +--|-- e7e6       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("too many values to unpack (expected %d)", "\377\271\261\214\216\213\231\224")
 c0bafb8:                               \--|--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb060 <mp_foo_bar+0x1d0>)

 c0bafba:                                  |  |  |  |  |  |  |  |  |  +--|-- e7e4       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("argument has wrong type", "\377\236\211\273\207")
 c0bafbc:                                  \--|--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb064 <mp_foo_bar+0x1d4>)
 c0bafbe:                                     |  |  |  |  |  |  |  |  +--|-- e7e2       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("type object '%q' has no attribute '%q'", "\377\207\200\203\211\222\206\203")
 c0bafc0:                                     \--|--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb068 <mp_foo_bar+0x1d8>)
 c0bafc2:                                        |  |  |  |  |  |  |  +--|-- e7e0       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("'%s' object has no attribute '%q'", "\377\201\200\211\222\206\203")
 c0bafc4:                                        \--|--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb06c <mp_foo_bar+0x1dc>)
 c0bafc6:                                           |  |  |  |  |  |  +--|-- e7de       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("'%s' object isn't iterable", "\377\201\200\202\256")
 c0bafc8:                                           \--|--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb070 <mp_foo_bar+0x1e0>)
 c0bafca:                                              |  |  |  |  |  +--|-- e7dc       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("'%s' object isn't an iterator", "\377\201\200\202\235\257")
 c0bafcc:                                              \--|--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb074 <mp_foo_bar+0x1e4>)
 c0bafce:                                                 |  |  |  |  +--|-- e7da       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("generator raised StopIteration", "\377\251\266\233")
 c0bafd0:                                                 \--|--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb078 <mp_foo_bar+0x1e8>)
 c0bafd2:                                                    |  |  |  +--|-- e7d8       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("exceptions must derive from BaseException", "\377\247\263\245\250\232")
 c0bafd4:                                                    \--|--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb07c <mp_foo_bar+0x1ec>)
 c0bafd6:                                                       |  |  +--|-- e7d6       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("can't import name %q", "\377\215\253\217\225")
 c0bafd8:                                                       \--|--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb080 <mp_foo_bar+0x1f0>)
 c0bafda:                                                          |  +--|-- e7d4       b.n     c0baf86 <mp_foo_bar+0xf6>
MP_MATCH_COMPRESSED("memory allocation failed, heap is locked", "\377\212\205\210\252\255\260")
 c0bafdc:                                                          \--|--|-> 4929       ldr     r1, [pc, #164]  @ (c0bb084 <mp_foo_bar+0x1f4>)
 c0bafde:                                                             \--|-- e7d2       b.n     c0baf86 <mp_foo_bar+0xf6>
 c0bafe0:                                                                \-> 4829       ldr     r0, [pc, #164]  @ (c0bb088 <mp_foo_bar+0x1f8>)
}
 c0bafe2:                                                                    b003       add     sp, #12
 c0bafe4:                                                                    f85d eb04  ldr.w   lr, [sp], #4
    return mp_obj_new_exception_msg(&mp_type_TypeError, MP_ERROR_TEXT("exceptions must derive from BaseException"));
 c0bafe8:                                                                    f033 b902  b.w     c0ee1f0 <mp_obj_new_exception_msg>
 c0bafec:                                                                    30007648   .word   0x30007648
 c0baff0:                                                                    0c1329c1   .word   0x0c1329c1
 c0baff4:                                                                    0c132b68   .word   0x0c132b68
 c0baff8:                                                                    0c1329d9   .word   0x0c1329d9
 c0baffc:                                                                    0c1329f1   .word   0x0c1329f1
 c0bb000:                                                                    0c132a0f   .word   0x0c132a0f
 c0bb004:                                                                    0c132a24   .word   0x0c132a24
 c0bb008:                                                                    0c15754b   .word   0x0c15754b
 c0bb00c:                                                                    0c132a49   .word   0x0c132a49
 c0bb010:                                                                    0c132a64   .word   0x0c132a64
 c0bb014:                                                                    0c132a87   .word   0x0c132a87
 c0bb018:                                                                    0c132aaf   .word   0x0c132aaf
 c0bb01c:                                                                    0c132ac7   .word   0x0c132ac7
 c0bb020:                                                                    0c132aee   .word   0x0c132aee
 c0bb024:                                                                    0c132b10   .word   0x0c132b10
 c0bb028:                                                                    0c132b2b   .word   0x0c132b2b
 c0bb02c:                                                                    0c132b49   .word   0x0c132b49
 c0bb030:                                                                    0c132b92   .word   0x0c132b92
 c0bb034:                                                                    0c132ba7   .word   0x0c132ba7
 c0bb038:                                                                    0c132bd0   .word   0x0c132bd0
 c0bb03c:                                                                    0c13296c   .word   0x0c13296c
 c0bb040:                                                                    0c1328f0   .word   0x0c1328f0
 c0bb044:                                                                    0c1328f6   .word   0x0c1328f6

Copy link

codecov bot commented Apr 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (6406afb) to head (f20c332).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17196   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21890    21890           
=======================================
  Hits        21572    21572           
  Misses        318      318           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@dlech
Copy link
Contributor

dlech commented Apr 27, 2025

This issue was reported in #7659 (comment)

The way I read that comment, it is talking about having to do a bunch of strcmp, not the actual implementation of the function itself.

Otherwise, it seems that the compiler doesn't optimize the strcmp

The code size report says this didn't change the code size on any of the common ports, so it seems like this change has no effect. Is it possible you are compiling with different optimization settings?

@agatti
Copy link
Contributor

agatti commented Apr 27, 2025

In GCC from version 4.8 (the oldest used to build current MicroPython, for the ESP8266 platform) and onwards, strcmp and __builtin_strcmp are treated the same by the compiler, so this line probably does nothing - see https://gcc.gnu.org/onlinedocs/gcc/Library-Builtins.html

I believe LLVM/Clang behaves the same way, so that wouldn't change much on macOS or Android builds too. Not sure about MSVC, it may have an intrinsic that wraps an x86/x64 opcode to perform that operation, but with a different name.

Also, __builtin_strcmp is not available in XC16 (see https://onlinedocs.microchip.com/oxy/GUID-C4E60FF5-3DAB-44F1-BA61-4BD962D8F469-en-US-2/GUID-4CC15F61-B02E-4B38-8611-2FD88B9780C1.html), so this may or may not break the pic16 build.

If you really want to stick with this then you should wrap this in a macro or do a bit of C preprocessing work beforehand, see how __builtin_popcount is used as an example (also in py/misc.h).

@romanz
Copy link
Author

romanz commented Apr 27, 2025

Is it possible you are compiling with different optimization settings?

Thanks! I will compare the optimization flags.

@romanz
Copy link
Author

romanz commented Apr 27, 2025

Our build is using -ffreestanding:

       -ffreestanding
           Assert  that compilation targets a freestanding environment.  This implies -fno-builtin.  A freestanding environment is one in which the standard
           library may not exist, and program startup may not necessarily be at "main".  The most obvious example is an OS kernel.  This  is  equivalent  to
           -fno-hosted.

And IIUC, -fno-builtin results in disabling the MICROPY_ROM_TEXT_COMPRESSION optimization (since it probably relies on strcmp being equivalent to __builtin_strcmp):

       -fno-builtin
       -fno-builtin-function
           Don't recognize built-in functions that do not begin with __builtin_ as prefix.

           GCC  normally  generates  special  code  to handle certain built-in functions more efficiently; for instance, calls to "alloca" may become single
           instructions which adjust the stack directly, and calls to "memcpy" may become inline copy loops.  The resulting code is often both  smaller  and
           faster, but since the function calls no longer appear as such, you cannot set a breakpoint on those calls, nor can you change the behavior of the
           functions  by linking with a different library.  In addition, when a function is recognized as a built-in function, GCC may use information about
           that function to warn about problems with calls to that function, or to generate more efficient code, even if the resulting code  still  contains
           calls  to that function.  For example, warnings are given with -Wformat for bad calls to "printf" when "printf" is built in and "strlen" is known
           not to modify global memory.

           With the -fno-builtin-function option only the built-in function function is disabled.  function must not begin with __builtin_.  If  a  function
           is  named  that is not built-in in this version of GCC, this option is ignored.  There is no corresponding -fbuiltin-function option; if you wish
           to enable built-in functions selectively when using -fno-builtin or -ffreestanding, you may define macros such as:

                   #define abs(n)          __builtin_abs ((n))
                   #define strcpy(d, s)    __builtin_strcpy ((d), (s))

@dlech
Copy link
Contributor

dlech commented Apr 27, 2025

Generally, MicroPython uses just -nostdlib, not -ffreestanding. Curious why do you need to use that for your build?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants