From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 8E3645818D1; Wed, 16 Aug 2023 20:04:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8E3645818D1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692205494; bh=LmYS/JiAQmA9UerEFpXh09ytdI7NOD/OyqZu/OFcc5w=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ALnCwaIAdFOPetNU6N5pY6FVjOFD/ufX+8+JiaOZdNMgGYgbfCNIq96yDXF1/kBUj 99w8nWnNFl8lTq1jUN6KcMDICoa4ibk4/T8VMCyY0AQEhjjVS5pl+iqCkC0dXRUeTv vNvfHlBw8KAf6tJ8CXu8ON8BoD8p3mrF+FwPOnNE= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [95.163.41.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DBC0458123F for ; Wed, 16 Aug 2023 20:04:53 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DBC0458123F Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1qWJwz-00Diro-0T; Wed, 16 Aug 2023 20:04:53 +0300 Message-ID: <0188f90a-9d0f-089c-9015-1e60f366ecc8@tarantool.org> Date: Wed, 16 Aug 2023 20:04:52 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Content-Language: en-US To: Maxim Kokryashkin , Sergey Kaplun References: <3ea79a870cded9d8cd90c9b930e965d2ca075200.1691592488.git.skaplun@tarantool.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9700E0DCE2907754D9B1C46D604AD036F8866CBFA63B460F7182A05F5380850403C7E6F5FB9B8E48E32BE871BD35DD6155F8A94AEB30A2A555A852A8C15758E5F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE776FF0B15B6DDC389C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7ACC115E2004B0725EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BE5CCB53A13BC8DBA41ACC3C07DEACD79166611F733F9AA08CC7F00164DA146DAFE8445B8C89999728AA50765F7900637933091C8A0E99779389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8062BEEFFB5F8EA3EF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C4E7D9683544204AF6136E347CC761E074AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3BFFF513DCDB8CC61BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166176DF2183F8FC7C0D2D576BCF940C736725E5C173C3A84C39D7D3120FB43BDE335872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A51EAB6D0B99E84E70B9C6BDD13B58F39BEF5B8AB05602438DF87CCE6106E1FC07E67D4AC08A07B9B0632EDEA9CD5989A3CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF6F054CE2946ECF6609EE762F7288955B9C799C5880AC7D83C6C3DD52D9CFC26248F49047001ACAA24F7434E9608BF9FE3B51DE906773C535160251A1361FE58A74DFFEFA5DC0E7F02C26D483E81D6BE0DBAE6F56676BC7117BB6831D7356A2DEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFRrmMqSMPxokLPsvouy7hQ== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F25884582D5012AC19FC39D3D0D6BAD4F3F4F576CA7FB22615CA2894282EC151BADDC1D3523A6D01B4765B2DFB59E2DDD9FE06B14FA522850F29BC30B0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 09/19] FFI: Eliminate hardcoded string hashes. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey Thanks for the patch! LGTM On 8/15/23 16:07, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the patch! > LGTM, except for a few comments below. > > On Wed, Aug 09, 2023 at 06:35:58PM +0300, Sergey Kaplun via Tarantool-patches wrote: >> From: Mike Pall >> >> (cherry-picked from commit 70f4b15ee45a6137fe6b48b941faea79d72f7159) >> >> This patch refactors FFI parsing of supported C attributes and pragmas, >> `ffi.abi()` parameter check. It replaces usage of comparison (with > Typo: s/usage/the usage/ >> hardcoded string hashes) with search in the given string with the > Typo: s/with search/with a search/ >> format: "\XXXattribute1\XXXattribute2", where `\XXX` is the length of >> "attribute" name. >> >> Sergey Kaplun: >> * added the description for the commit >> >> Part of tarantool/tarantool#8825 >> --- >> src/lib_ffi.c | 35 ++++++++++------------ >> src/lj_cparse.c | 77 +++++++++++++++++++++++++++++++------------------ >> src/lj_cparse.h | 2 ++ >> 3 files changed, 67 insertions(+), 47 deletions(-) >> >> diff --git a/src/lib_ffi.c b/src/lib_ffi.c >> index d1fe1a14..62af54c1 100644 >> --- a/src/lib_ffi.c >> +++ b/src/lib_ffi.c >> @@ -720,50 +720,47 @@ LJLIB_CF(ffi_fill) LJLIB_REC(.) >> return 0; >> } >> >> -#define H_(le, be) LJ_ENDIAN_SELECT(0x##le, 0x##be) >> - >> /* Test ABI string. */ >> LJLIB_CF(ffi_abi) LJLIB_REC(.) >> { >> GCstr *s = lj_lib_checkstr(L, 1); >> - int b = 0; >> - switch (s->hash) { >> + int b = lj_cparse_case(s, >> #if LJ_64 >> - case H_(849858eb,ad35fd06): b = 1; break; /* 64bit */ >> + "\00564bit" >> #else >> - case H_(662d3c79,d0e22477): b = 1; break; /* 32bit */ >> + "\00532bit" >> #endif >> #if LJ_ARCH_HASFPU >> - case H_(e33ee463,e33ee463): b = 1; break; /* fpu */ >> + "\003fpu" >> #endif >> #if LJ_ABI_SOFTFP >> - case H_(61211a23,c2e8c81c): b = 1; break; /* softfp */ >> + "\006softfp" >> #else >> - case H_(539417a8,8ce0812f): b = 1; break; /* hardfp */ >> + "\006hardfp" >> #endif >> #if LJ_ABI_EABI >> - case H_(2182df8f,f2ed1152): b = 1; break; /* eabi */ >> + "\004eabi" >> #endif >> #if LJ_ABI_WIN >> - case H_(4ab624a8,4ab624a8): b = 1; break; /* win */ >> + "\003win" >> #endif >> #if LJ_TARGET_UWP >> - case H_(a40f0bcb,a40f0bcb): b = 1; break; /* uwp */ >> + "\003uwp" >> +#endif >> +#if LJ_LE >> + "\002le" >> +#else >> + "\002be" >> #endif >> - case H_(3af93066,1f001464): b = 1; break; /* le/be */ >> #if LJ_GC64 >> - case H_(9e89d2c9,13c83c92): b = 1; break; /* gc64 */ >> + "\004gc64" >> #endif >> - default: >> - break; >> - } >> + ) >= 0; >> setboolV(L->top-1, b); >> setboolV(&G(L)->tmptv2, b); /* Remember for trace recorder. */ >> return 1; >> } >> >> -#undef H_ >> - >> LJLIB_PUSH(top-8) LJLIB_SET(!) /* Store reference to miscmap table. */ >> >> LJLIB_CF(ffi_metatype) >> diff --git a/src/lj_cparse.c b/src/lj_cparse.c >> index fb440567..07c643d4 100644 >> --- a/src/lj_cparse.c >> +++ b/src/lj_cparse.c >> @@ -28,6 +28,24 @@ >> ** If in doubt, please check the input against your favorite C compiler. >> */ >> >> +/* -- Miscellaneous ------------------------------------------------------- */ >> + >> +/* Match string against a C literal. */ >> +#define cp_str_is(str, k) \ >> + ((str)->len == sizeof(k)-1 && !memcmp(strdata(str), k, sizeof(k)-1)) >> + >> +/* Check string against a linear list of matches. */ >> +int lj_cparse_case(GCstr *str, const char *match) >> +{ >> + MSize len; >> + int n; >> + for (n = 0; (len = (MSize)*match++); n++, match += len) { >> + if (str->len == len && !memcmp(match, strdata(str), len)) >> + return n; >> + } >> + return -1; >> +} >> + >> /* -- C lexer ------------------------------------------------------------- */ >> >> /* C lexer token names. */ >> @@ -930,8 +948,6 @@ static CTypeID cp_decl_intern(CPState *cp, CPDecl *decl) >> >> /* -- C declaration parser ------------------------------------------------ */ >> >> -#define H_(le, be) LJ_ENDIAN_SELECT(0x##le, 0x##be) >> - >> /* Reset declaration state to declaration specifier. */ >> static void cp_decl_reset(CPDecl *decl) >> { >> @@ -1071,44 +1087,57 @@ static void cp_decl_gccattribute(CPState *cp, CPDecl *decl) >> attrstr = lj_str_new(cp->L, c+2, attrstr->len-4); >> #endif >> cp_next(cp); >> - switch (attrstr->hash) { >> - case H_(64a9208e,8ce14319): case H_(8e6331b2,95a282af): /* aligned */ >> + switch (lj_cparse_case(attrstr, >> + "\007aligned" "\013__aligned__" >> + "\006packed" "\012__packed__" >> + "\004mode" "\010__mode__" >> + "\013vector_size" "\017__vector_size__" >> +#if LJ_TARGET_X86 >> + "\007regparm" "\013__regparm__" >> + "\005cdecl" "\011__cdecl__" >> + "\010thiscall" "\014__thiscall__" >> + "\010fastcall" "\014__fastcall__" >> + "\007stdcall" "\013__stdcall__" >> + "\012sseregparm" "\016__sseregparm__" >> +#endif >> + )) { >> + case 0: case 1: /* aligned */ >> cp_decl_align(cp, decl); >> break; >> - case H_(42eb47de,f0ede26c): case H_(29f48a09,cf383e0c): /* packed */ >> + case 2: case 3: /* packed */ >> decl->attr |= CTFP_PACKED; >> break; >> - case H_(0a84eef6,8dfab04c): case H_(995cf92c,d5696591): /* mode */ >> + case 4: case 5: /* mode */ >> cp_decl_mode(cp, decl); >> break; >> - case H_(0ab31997,2d5213fa): case H_(bf875611,200e9990): /* vector_size */ >> + case 6: case 7: /* vector_size */ >> { >> CTSize vsize = cp_decl_sizeattr(cp); >> if (vsize) CTF_INSERT(decl->attr, VSIZEP, lj_fls(vsize)); >> } >> break; >> #if LJ_TARGET_X86 >> - case H_(5ad22db8,c689b848): case H_(439150fa,65ea78cb): /* regparm */ >> + case 8: case 9: /* regparm */ >> CTF_INSERT(decl->fattr, REGPARM, cp_decl_sizeattr(cp)); >> decl->fattr |= CTFP_CCONV; >> break; >> - case H_(18fc0b98,7ff4c074): case H_(4e62abed,0a747424): /* cdecl */ >> + case 10: case 11: /* cdecl */ >> CTF_INSERT(decl->fattr, CCONV, CTCC_CDECL); >> decl->fattr |= CTFP_CCONV; >> break; >> - case H_(72b2e41b,494c5a44): case H_(f2356d59,f25fc9bd): /* thiscall */ >> + case 12: case 13: /* thiscall */ >> CTF_INSERT(decl->fattr, CCONV, CTCC_THISCALL); >> decl->fattr |= CTFP_CCONV; >> break; >> - case H_(0d0ffc42,ab746f88): case H_(21c54ba1,7f0ca7e3): /* fastcall */ >> + case 14: case 15: /* fastcall */ >> CTF_INSERT(decl->fattr, CCONV, CTCC_FASTCALL); >> decl->fattr |= CTFP_CCONV; >> break; >> - case H_(ef76b040,9412e06a): case H_(de56697b,c750e6e1): /* stdcall */ >> + case 16: case 17: /* stdcall */ >> CTF_INSERT(decl->fattr, CCONV, CTCC_STDCALL); >> decl->fattr |= CTFP_CCONV; >> break; >> - case H_(ea78b622,f234bd8e): case H_(252ffb06,8d50f34b): /* sseregparm */ >> + case 18: case 19: /* sseregparm */ >> decl->fattr |= CTF_SSEREGPARM; >> decl->fattr |= CTFP_CCONV; >> break; >> @@ -1140,16 +1169,13 @@ static void cp_decl_msvcattribute(CPState *cp, CPDecl *decl) >> while (cp->tok == CTOK_IDENT) { >> GCstr *attrstr = cp->str; >> cp_next(cp); >> - switch (attrstr->hash) { >> - case H_(bc2395fa,98f267f8): /* align */ >> + if (cp_str_is(attrstr, "align")) { >> cp_decl_align(cp, decl); >> - break; >> - default: /* Ignore all other attributes. */ >> + } else { /* Ignore all other attributes. */ >> if (cp_opt(cp, '(')) { >> while (cp->tok != ')' && cp->tok != CTOK_EOF) cp_next(cp); >> cp_check(cp, ')'); >> } >> - break; >> } >> } >> cp_check(cp, ')'); >> @@ -1729,17 +1755,16 @@ static CTypeID cp_decl_abstract(CPState *cp) >> static void cp_pragma(CPState *cp, BCLine pragmaline) >> { >> cp_next(cp); >> - if (cp->tok == CTOK_IDENT && >> - cp->str->hash == H_(e79b999f,42ca3e85)) { /* pack */ >> + if (cp->tok == CTOK_IDENT && cp_str_is(cp->str, "pack")) { >> cp_next(cp); >> cp_check(cp, '('); >> if (cp->tok == CTOK_IDENT) { >> - if (cp->str->hash == H_(738e923c,a1b65954)) { /* push */ >> + if (cp_str_is(cp->str, "push")) { >> if (cp->curpack < CPARSE_MAX_PACKSTACK) { >> cp->packstack[cp->curpack+1] = cp->packstack[cp->curpack]; >> cp->curpack++; >> } >> - } else if (cp->str->hash == H_(6c71cf27,6c71cf27)) { /* pop */ >> + } else if (cp_str_is(cp->str, "pop")) { >> if (cp->curpack > 0) cp->curpack--; >> } else { >> cp_errmsg(cp, cp->tok, LJ_ERR_XSYMBOL); >> @@ -1788,13 +1813,11 @@ static void cp_decl_multi(CPState *cp) >> if (tok == CTOK_INTEGER) { >> cp_line(cp, hashline); >> continue; >> - } else if (tok == CTOK_IDENT && >> - cp->str->hash == H_(187aab88,fcb60b42)) { /* line */ >> + } else if (tok == CTOK_IDENT && cp_str_is(cp->str, "line")) { >> if (cp_next(cp) != CTOK_INTEGER) cp_err_token(cp, tok); >> cp_line(cp, hashline); >> continue; >> - } else if (tok == CTOK_IDENT && >> - cp->str->hash == H_(f5e6b4f8,1d509107)) { /* pragma */ >> + } else if (tok == CTOK_IDENT && cp_str_is(cp->str, "pragma")) { >> cp_pragma(cp, hashline); >> continue; >> } else { >> @@ -1865,8 +1888,6 @@ static void cp_decl_single(CPState *cp) >> if (cp->tok != CTOK_EOF) cp_err_token(cp, CTOK_EOF); >> } >> >> -#undef H_ >> - >> /* ------------------------------------------------------------------------ */ >> >> /* Protected callback for C parser. */ >> diff --git a/src/lj_cparse.h b/src/lj_cparse.h >> index bad1060b..e40b4047 100644 >> --- a/src/lj_cparse.h >> +++ b/src/lj_cparse.h >> @@ -60,6 +60,8 @@ typedef struct CPState { >> >> LJ_FUNC int lj_cparse(CPState *cp); >> >> +LJ_FUNC int lj_cparse_case(GCstr *str, const char *match); >> + >> #endif >> >> #endif >> -- >> 2.41.0 >>