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 DE1FB5C571F; Tue, 15 Aug 2023 16:07:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DE1FB5C571F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692104865; bh=BnGzna+9K94XtSkdD1Ocf1r5xxQeQrlSbCFv9cYPdPY=; 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=AHd5fd4y3yKrPjTCHn6e7R7fkMV9vYVuEhz0U/pXFFrJq77mdQyLYSx7BqHgscw+j jD/nTaY0OlaskrjXGGYoW/GcFrPWwrIycvXb+uIIxfgqjfkRsX69XFRjeJ0HE9l7lU TF2hhfD6maqi44ONvYi9HpMeIYonjpr+qdE59A3M= Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [95.163.41.68]) (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 53B86589F42 for ; Tue, 15 Aug 2023 16:07:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 53B86589F42 Received: by smtp29.i.mail.ru with esmtpa (envelope-from ) id 1qVtlu-0077Z4-1D; Tue, 15 Aug 2023 16:07:42 +0300 Date: Tue, 15 Aug 2023 16:07:41 +0300 To: Sergey Kaplun Message-ID: References: <3ea79a870cded9d8cd90c9b930e965d2ca075200.1691592488.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3ea79a870cded9d8cd90c9b930e965d2ca075200.1691592488.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD94C9043905E7830E1A8B228FB8AC5501C05F944AC2BE2832B182A05F5380850404C228DA9ACA6FE27890C17B898E013C58760035DA98949515AE2ACE483BA2CCDEAAFE5DDC2494280 X-C1DE0DAB: 0D63561A33F958A5A87511EE5EFB3F3FB92582003CD23D4E3FE6D546EE1A1C12F87CCE6106E1FC07E67D4AC08A07B9B04CB6874B0BCFF0B8CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0AD5177F0B940C8B66ECE892A7B2722663E91682638B966EB3F662256BEEFA9527F4A6A5CA4B854B397D551A818FE19E664007E325ADCA6A3DBEB0801132344FF546004024B1CD5661137FD76D11AF80A0DDAFAE71BB1B6EC64BCA349E5ABE11994EA455F16B58544A21C197AAF4D2E4732965026E5D17F6739C77C69D99B9914278E50E1F0597A6FD5CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojHVl7ekwB6hgqYDKw9qE24A== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE407A1D9DDF988FAAC519DA8DF39701B352BEFD9FE33A6421891D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 >