From 7563a5f46408a01267093bae36cd1de204ff990c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Mon, 10 Jun 2019 10:30:26 +0200 Subject: genop.tab: Insert an "OTP 23" comment for clarity --- lib/compiler/src/genop.tab | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/compiler/src/genop.tab b/lib/compiler/src/genop.tab index 03507bafb3..0a38d17857 100755 --- a/lib/compiler/src/genop.tab +++ b/lib/compiler/src/genop.tab @@ -597,6 +597,8 @@ BEAM_FORMAT_NUMBER=0 ## @doc Sets the current position of Ctx to Pos 168: bs_set_position/2 +# OTP 23 + ## @spec swap Register1 Register2 ## @doc Swaps the contents of two registers. 169: swap/2 -- cgit v1.2.3 From 0d6353767dab33f2b676d7f30c8c2f27606c02f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 13 Jun 2019 11:56:11 +0200 Subject: Stop supporting decoding of old funs in the external term format The new external format for funs (NEW_FUN_EXT) was introduced in OTP R8 along with the `make_fun2` instruction. Therefore, it should be safe to stop decoding the old FUN_EXT tag. --- erts/emulator/beam/erl_fun.c | 21 ----------- erts/emulator/beam/erl_fun.h | 1 - erts/emulator/beam/external.c | 76 ++++----------------------------------- erts/emulator/test/hash_SUITE.erl | 33 +++++++---------- 4 files changed, 19 insertions(+), 112 deletions(-) diff --git a/erts/emulator/beam/erl_fun.c b/erts/emulator/beam/erl_fun.c index 9c866250bb..8e4372f11e 100644 --- a/erts/emulator/beam/erl_fun.c +++ b/erts/emulator/beam/erl_fun.c @@ -99,27 +99,6 @@ int erts_fun_table_sz(void) return sz; } -ErlFunEntry* -erts_put_fun_entry(Eterm mod, int uniq, int index) -{ - ErlFunEntry template; - ErlFunEntry* fe; - erts_aint_t refc; - ASSERT(is_atom(mod)); - template.old_uniq = uniq; - template.old_index = index; - template.module = mod; - erts_fun_write_lock(); - fe = (ErlFunEntry *) hash_put(&erts_fun_table, (void*) &template); - sys_memset(fe->uniq, 0, sizeof(fe->uniq)); - fe->index = 0; - refc = erts_refc_inctest(&fe->refc, 0); - if (refc < 2) /* New or pending delete */ - erts_refc_inc(&fe->refc, 1); - erts_fun_write_unlock(); - return fe; -} - ErlFunEntry* erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index, byte* uniq, int index, int arity) diff --git a/erts/emulator/beam/erl_fun.h b/erts/emulator/beam/erl_fun.h index fb2901d866..eefc7a95bb 100644 --- a/erts/emulator/beam/erl_fun.h +++ b/erts/emulator/beam/erl_fun.h @@ -74,7 +74,6 @@ void erts_init_fun_table(void); void erts_fun_info(fmtfn_t, void *); int erts_fun_table_sz(void); -ErlFunEntry* erts_put_fun_entry(Eterm mod, int uniq, int index); ErlFunEntry* erts_get_fun_entry(Eterm mod, int uniq, int index); ErlFunEntry* erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index, diff --git a/erts/emulator/beam/external.c b/erts/emulator/beam/external.c index ec67ab2aed..8a8e62a608 100644 --- a/erts/emulator/beam/external.c +++ b/erts/emulator/beam/external.c @@ -4011,73 +4011,6 @@ dec_term_atom_common: next = &(funp->creator); break; } - case FUN_EXT: - { - ErlFunThing* funp = (ErlFunThing *) hp; - Eterm module; - Sint old_uniq; - Sint old_index; - unsigned num_free; - int i; - Eterm temp; - - num_free = get_int32(ep); - ep += 4; - hp += ERL_FUN_SIZE; - hp += num_free; - factory->hp = hp; - funp->thing_word = HEADER_FUN; - funp->num_free = num_free; - *objp = make_fun(funp); - - /* Creator pid */ - if ((*ep != PID_EXT && *ep != NEW_PID_EXT) - || (ep = dec_pid(edep, factory, ep+1, - &funp->creator, *ep))==NULL) { - goto error; - } - - /* Module */ - if ((ep = dec_atom(edep, ep, &module)) == NULL) { - goto error; - } - - /* Index */ - if ((ep = dec_term(edep, factory, ep, &temp, NULL)) == NULL) { - goto error; - } - if (!is_small(temp)) { - goto error; - } - old_index = unsigned_val(temp); - - /* Uniq */ - if ((ep = dec_term(edep, factory, ep, &temp, NULL)) == NULL) { - goto error; - } - if (!is_small(temp)) { - goto error; - } - - /* - * It is safe to link the fun into the fun list only when - * no more validity tests can fail. - */ - funp->next = factory->off_heap->first; - factory->off_heap->first = (struct erl_off_heap_header*)funp; - old_uniq = unsigned_val(temp); - - funp->fe = erts_put_fun_entry(module, old_uniq, old_index); - funp->arity = funp->fe->address[-1] - num_free; - hp = factory->hp; - - /* Environment */ - for (i = num_free-1; i >= 0; i--) { - funp->env[i] = (Eterm) next; - next = funp->env + i; - } - break; - } case ATOM_INTERNAL_REF2: n = get_int16(ep); ep += 2; @@ -4836,9 +4769,6 @@ init_done: total_size = get_int32(ep); CHKSIZE(total_size); ep += 1+16+4+4; - /*FALLTHROUGH*/ - - case FUN_EXT: CHKSIZE(4); num_free = get_int32(ep); ep += 4; @@ -4849,6 +4779,12 @@ init_done: heap_size += ERL_FUN_SIZE + num_free; break; } + case FUN_EXT: + /* + * OTP 23: No longer support decoding the old fun + * representation. + */ + goto error; case ATOM_INTERNAL_REF2: SKIP(2+atom_extra_skip); atom_extra_skip = 0; diff --git a/erts/emulator/test/hash_SUITE.erl b/erts/emulator/test/hash_SUITE.erl index 1bf9e033bf..dd71c3da58 100644 --- a/erts/emulator/test/hash_SUITE.erl +++ b/erts/emulator/test/hash_SUITE.erl @@ -223,26 +223,17 @@ basic_test() -> 16#77777777777777],16#FFFFFFFF), ExternalReference = <<131,114,0,3,100,0,13,110,111,110,111,100,101,64, 110,111,104,111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0>>, - 1113403635 = erlang:phash(binary_to_term(ExternalReference), - 16#FFFFFFFF), - ExternalFun = <<131,117,0,0,0,3,103,100,0,13,110,111,110,111,100,101,64, - 110,111,104,111,115,116,0,0,0,38,0,0,0,0,0,100,0,8,101, - 114,108,95,101,118,97,108,97,20,98,5,182,139,98,108,0,0, - 0,3,104,2,100,0,1,66,109,0,0,0,33,131,114,0,3,100,0,13, - 110,111,110,111,100,101,64,110,111,104,111,115,116,0,0, - 0,0,122,0,0,0,0,0,0,0,0,104,2,100,0,1,76,107,0,33,131, - 114,0,3,100,0,13,110,111,110,111,100,101,64,110,111,104, - 111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0,104,2,100,0,1,82, - 114,0,3,100,0,13,110,111,110,111,100,101,64,110,111,104, - 111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0,106,108,0,0,0,1, - 104,5,100,0,6,99,108,97,117,115,101,97,1,106,106,108,0,0, - 0,1,104,3,100,0,7,105,110,116,101,103,101,114,97,1,97,1, - 106,106,104,3,100,0,4,101,118,97,108,104,2,100,0,5,115, - 104,101,108,108,100,0,10,108,111,99,97,108,95,102,117, - 110,99,108,0,0,0,1,103,100,0,13,110,111,110,111,100,101, - 64,110,111,104,111,115,116,0,0,0,22,0,0,0,0,0,106>>, - 170987488 = erlang:phash(binary_to_term(ExternalFun), - 16#FFFFFFFF), + ExternalReference = <<131,114,0,3,100,0,13,110,111,110,111,100,101,64, + 110,111,104,111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0>>, + 1113403635 = phash_from_external(ExternalReference), + + ExternalFun = <<131,112,0,0,0,70,1,212,190,220,28,179,144,194,131, + 19,215,105,97,77,251,125,93,0,0,0,0,0,0,0,2,100,0,1, + 116,97,0,98,6,165,246,224,103,100,0,13,110,111, + 110,111,100,101,64,110,111,104,111,115,116,0,0,0,91, + 0,0,0,0,0,97,2,97,1>>, + 25769064 = phash_from_external(ExternalFun), + case (catch erlang:phash(1,0)) of {'EXIT',{badarg, _}} -> ok; @@ -250,6 +241,8 @@ basic_test() -> exit(phash_accepted_zero_as_range) end. +phash_from_external(Ext) -> + erlang:phash(binary_to_term(Ext), 16#FFFFFFFF). range_test() -> F = fun(From,From,_FF) -> -- cgit v1.2.3 From 8fde098f9d98d227a6609f77419369cb50ad701b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 4 Jun 2019 06:33:34 +0200 Subject: hipe: Use the new index when translating funs OTP 23 will start using the new index instead of the old index when comparing funs. Update the translator to icode accordingly. --- lib/hipe/icode/hipe_beam_to_icode.erl | 43 ++++------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/lib/hipe/icode/hipe_beam_to_icode.erl b/lib/hipe/icode/hipe_beam_to_icode.erl index 42e4ead169..1246af1da3 100644 --- a/lib/hipe/icode/hipe_beam_to_icode.erl +++ b/lib/hipe/icode/hipe_beam_to_icode.erl @@ -689,8 +689,8 @@ trans_fun([{call_fun,N}|Instructions], Env) -> Dst = [mk_var({r,0})], [hipe_icode:mk_comment('call_fun'), hipe_icode:mk_primop(Dst,call_fun,Args) | trans_fun(Instructions,Env)]; -%%--- patched_make_fun --- make_fun/make_fun2 after fixes -trans_fun([{patched_make_fun,MFA,Magic,FreeVarNum,Index}|Instructions], Env) -> +%%--- make_fun2 --- +trans_fun([{make_fun2,MFA,Index,Magic,FreeVarNum}|Instructions], Env) -> Args = extract_fun_args(FreeVarNum), Dst = [mk_var({r,0})], Fun = hipe_icode:mk_primop(Dst, @@ -1957,7 +1957,7 @@ mod_find_closure_info([FunCode|Fs], CI) -> mod_find_closure_info([], CI) -> CI. -find_closure_info([{patched_make_fun,MFA={_M,_F,A},_Magic,FreeVarNum,_Index}|BeamCode], +find_closure_info([{make_fun2,{_M,_F,A}=MFA,_Index,_Magic,FreeVarNum}|BeamCode], ClosureInfo) -> NewClosure = %% A-FreeVarNum+1 (The real arity + 1 for the closure) #closure_info{mfa=MFA, arity=A-FreeVarNum+1, fv_arity=FreeVarNum}, @@ -2035,41 +2035,8 @@ split_params(N, [ArgN|OrgArgs], Args) -> %%----------------------------------------------------------------------- preprocess_code(ModuleCode) -> - PatchedCode = patch_R7_funs(ModuleCode), - ClosureInfo = find_closure_info(PatchedCode), - {PatchedCode, ClosureInfo}. - -%%----------------------------------------------------------------------- -%% Patches the "make_fun" BEAM instructions of R7 so that they also -%% contain the index that the BEAM loader generates for funs. -%% -%% The index starts from 0 and is incremented by 1 for each make_fun -%% instruction encountered. -%% -%% Retained only for compatibility with BEAM code prior to R8. -%% -%% Temporarily, it also rewrites R8-PRE-RELEASE "make_fun2" -%% instructions, since their embedded indices don't work. -%%----------------------------------------------------------------------- - -patch_R7_funs(ModuleCode) -> - patch_make_funs(ModuleCode, 0). - -patch_make_funs([FunCode0|Fs], FunIndex0) -> - {PatchedFunCode,FunIndex} = patch_make_funs(FunCode0, FunIndex0, []), - [PatchedFunCode|patch_make_funs(Fs, FunIndex)]; -patch_make_funs([], _) -> []. - -patch_make_funs([{make_fun,MFA,Magic,FreeVarNum}|Is], FunIndex, Acc) -> - Patched = {patched_make_fun,MFA,Magic,FreeVarNum,FunIndex}, - patch_make_funs(Is, FunIndex+1, [Patched|Acc]); -patch_make_funs([{make_fun2,MFA,_BogusIndex,Magic,FreeVarNum}|Is], FunIndex, Acc) -> - Patched = {patched_make_fun,MFA,Magic,FreeVarNum,FunIndex}, - patch_make_funs(Is, FunIndex+1, [Patched|Acc]); -patch_make_funs([I|Is], FunIndex, Acc) -> - patch_make_funs(Is, FunIndex, [I|Acc]); -patch_make_funs([], FunIndex, Acc) -> - {lists:reverse(Acc),FunIndex}. + ClosureInfo = find_closure_info(ModuleCode), + {ModuleCode, ClosureInfo}. %%----------------------------------------------------------------------- -- cgit v1.2.3 From 0ddddac7691521f76d984140ec5d82db5ab1e3ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 4 Jun 2019 13:11:58 +0200 Subject: erl_fun.c: Remove unused struct definition --- erts/emulator/beam/erl_fun.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/erts/emulator/beam/erl_fun.c b/erts/emulator/beam/erl_fun.c index 8e4372f11e..ca1e6bf5e7 100644 --- a/erts/emulator/beam/erl_fun.c +++ b/erts/emulator/beam/erl_fun.c @@ -123,13 +123,6 @@ erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index, return fe; } -struct my_key { - Eterm mod; - byte* uniq; - int index; - ErlFunEntry* fe; -}; - ErlFunEntry* erts_get_fun_entry(Eterm mod, int uniq, int index) { -- cgit v1.2.3 From 5943ff2cfb38f7ad572bb76abf425dd128dab72d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 4 Jun 2019 12:51:54 +0200 Subject: Support sharing of fun entries in the runtime system Before OTP 23, there had to be a one-to-one correspodence between `make_fun2` instructions and fun entries. In the OTP 23 runtime system, start keying the fun entries with the explicit ("new") index included in the fun entry. That will allow multiple `make_fun2` instructions to refer to the same entry in the fun table. This change is safe for code compiled by OTP R15B or later, because the old and new indices are identical. The loader will refuse to load code where the old and new indices are not equal unless the code is known to be compiled with an OTP 23 compiler. --- erts/emulator/beam/beam_load.c | 42 ++++++++++++++++++++++++++++++++++++++---- erts/emulator/beam/erl_fun.c | 26 +++++++++++++++++++------- erts/emulator/beam/utils.c | 10 +++++----- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index 0de694f449..9add87d944 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -315,6 +315,7 @@ typedef struct LoaderState { * (or 0 if there is no on_load function) */ int otp_20_or_higher; /* Compiled with OTP 20 or higher */ + unsigned max_opcode; /* Highest opcode used in module */ /* * Atom table. @@ -1588,6 +1589,17 @@ static int read_lambda_table(LoaderState* stp) { unsigned int i; + unsigned int otp_22_or_lower; + + /* + * Determine whether this module was compiled with OTP 22 or lower + * by looking at the max opcode number. The compiler in OTP 23 will + * always set the max opcode to the opcode for `swap` (whether + * actually used or not) so that a module compiled for OTP 23 + * cannot be loaded in earlier versions. + */ + + otp_22_or_lower = stp->max_opcode < genop_swap_2; GetInt(stp, 4, stp->num_lambdas); if (stp->num_lambdas > stp->lambdas_allocated) { @@ -1619,6 +1631,29 @@ read_lambda_table(LoaderState* stp) GetInt(stp, 4, Index); GetInt(stp, 4, stp->lambdas[i].num_free); GetInt(stp, 4, OldUniq); + + /* + * Fun entries are now keyed by the explicit ("new") index in + * the fun entry. That allows multiple make_fun2 instructions + * to share the same fun entry (when the `fun F/A` syntax is + * used). Before OTP 23, fun entries were keyed by the old + * index, which is the order of the entries in the fun + * chunk. Each make_fun2 needed to refer to its own fun entry. + * + * Modules compiled before OTP 23 can safely be loaded if the + * old index and the new index are equal. That is true for all + * modules compiled with OTP R15 and later. + */ + if (otp_22_or_lower && i != Index) { + /* + * Compiled with a compiler before OTP R15B. The new indices + * are not reliable, so it is not safe to load this module. + */ + LoadError2(stp, "please re-compile this module with an " + ERLANG_OTP_RELEASE " compiler " + "(old-style fun with indices: %d/%d)", + i, Index); + } fe = erts_put_fun_entry2(stp->module, OldUniq, i, stp->mod_md5, Index, arity-stp->lambdas[i].num_free); stp->lambdas[i].fe = fe; @@ -1839,7 +1874,6 @@ read_code_header(LoaderState* stp) { unsigned head_size; unsigned version; - unsigned opcode_max; int i; /* @@ -1871,8 +1905,8 @@ read_code_header(LoaderState* stp) /* * Verify the number of the highest opcode used. */ - GetInt(stp, 4, opcode_max); - if (opcode_max > MAX_GENERIC_OPCODE) { + GetInt(stp, 4, stp->max_opcode); + if (stp->max_opcode > MAX_GENERIC_OPCODE) { LoadError2(stp, "This BEAM file was compiled for a later version" " of the run-time system than " ERLANG_OTP_RELEASE ".\n" @@ -1880,7 +1914,7 @@ read_code_header(LoaderState* stp) ERLANG_OTP_RELEASE " compiler.\n" " (Use of opcode %d; this emulator supports " "only up to %d.)", - opcode_max, MAX_GENERIC_OPCODE); + stp->max_opcode, MAX_GENERIC_OPCODE); } GetInt(stp, 4, stp->num_labels); diff --git a/erts/emulator/beam/erl_fun.c b/erts/emulator/beam/erl_fun.c index ca1e6bf5e7..257f9bf5b3 100644 --- a/erts/emulator/beam/erl_fun.c +++ b/erts/emulator/beam/erl_fun.c @@ -109,12 +109,12 @@ erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index, ASSERT(is_atom(mod)); template.old_uniq = old_uniq; - template.old_index = old_index; + template.index = index; template.module = mod; erts_fun_write_lock(); fe = (ErlFunEntry *) hash_put(&erts_fun_table, (void*) &template); sys_memcpy(fe->uniq, uniq, sizeof(fe->uniq)); - fe->index = index; + fe->old_index = old_index; fe->arity = arity; refc = erts_refc_inctest(&fe->refc, 0); if (refc < 2) /* New or pending delete */ @@ -131,7 +131,7 @@ erts_get_fun_entry(Eterm mod, int uniq, int index) ASSERT(is_atom(mod)); template.old_uniq = uniq; - template.old_index = index; + template.index = index; template.module = mod; erts_fun_read_lock(); ret = (ErlFunEntry *) hash_get(&erts_fun_table, (void*) &template); @@ -287,15 +287,27 @@ erts_dump_fun_entries(fmtfn_t to, void *to_arg) static HashValue fun_hash(ErlFunEntry* obj) { - return (HashValue) (obj->old_uniq ^ obj->old_index ^ atom_val(obj->module)); + return (HashValue) (obj->old_uniq ^ obj->index ^ atom_val(obj->module)); } static int fun_cmp(ErlFunEntry* obj1, ErlFunEntry* obj2) { - return !(obj1->module == obj2->module && + /* + * OTP 23: Use 'index' (instead of 'old_index') when comparing fun + * entries. In OTP 23, multiple make_fun2 instructions may refer to the + * the same 'index' (for the wrapper function generated for the + * 'fun F/A' syntax). + * + * This is safe when loading code compiled with OTP R15 and later, + * because since R15 (2011), the 'index' has been reliably equal + * to 'old_index'. The loader refuses to load modules compiled before + * OTP R15. + */ + + return !(obj1->module == obj2->module && obj1->old_uniq == obj2->old_uniq && - obj1->old_index == obj2->old_index); + obj1->index == obj2->index); } static ErlFunEntry* @@ -305,7 +317,7 @@ fun_alloc(ErlFunEntry* template) sizeof(ErlFunEntry)); obj->old_uniq = template->old_uniq; - obj->old_index = template->old_index; + obj->index = template->index; obj->module = template->module; erts_refc_init(&obj->refc, -1); obj->address = unloaded_fun; diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c index 88cdcc2675..ad71828d72 100644 --- a/erts/emulator/beam/utils.c +++ b/erts/emulator/beam/utils.c @@ -907,7 +907,7 @@ tail_recur: hash = hash * FUNNY_NUMBER10 + num_free; hash = hash*FUNNY_NUMBER1 + (atom_tab(atom_val(funp->fe->module))->slot.bucket.hvalue); - hash = hash*FUNNY_NUMBER2 + funp->fe->old_index; + hash = hash*FUNNY_NUMBER2 + funp->fe->index; hash = hash*FUNNY_NUMBER2 + funp->fe->old_uniq; if (num_free > 0) { if (num_free > 1) { @@ -1636,7 +1636,7 @@ make_hash2_helper(Eterm term_param, const int can_trap, Eterm* state_mref_write_ atom_tab(atom_val(funp->fe->module))->slot.bucket.hvalue, HCONST); UINT32_HASH_2 - (funp->fe->old_index, funp->fe->old_uniq, HCONST); + (funp->fe->index, funp->fe->old_uniq, HCONST); if (ctx.num_free == 0) { goto hash2_common; } else { @@ -2160,7 +2160,7 @@ make_internal_hash(Eterm term, Uint32 salt) ErlFunThing* funp = (ErlFunThing *) fun_val(term); Uint num_free = funp->num_free; UINT32_HASH_2(num_free, funp->fe->module, HCONST_20); - UINT32_HASH_2(funp->fe->old_index, funp->fe->old_uniq, HCONST_21); + UINT32_HASH_2(funp->fe->index, funp->fe->old_uniq, HCONST_21); if (num_free == 0) { goto pop_next; } else { @@ -2810,7 +2810,7 @@ tailrecur_ne: f1 = (ErlFunThing *) fun_val(a); f2 = (ErlFunThing *) fun_val(b); if (f1->fe->module != f2->fe->module || - f1->fe->old_index != f2->fe->old_index || + f1->fe->index != f2->fe->index || f1->fe->old_uniq != f2->fe->old_uniq || f1->num_free != f2->num_free) { goto not_equal; @@ -3405,7 +3405,7 @@ tailrecur_ne: if (diff != 0) { RETURN_NEQ(diff); } - diff = f1->fe->old_index - f2->fe->old_index; + diff = f1->fe->index - f2->fe->index; if (diff != 0) { RETURN_NEQ(diff); } -- cgit v1.2.3 From fccac1598cc13fcd9a3d7e85e2c4cae722f1bfc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 4 Jun 2019 12:52:34 +0200 Subject: Create a shared wrapper function for all occurrences of 'fun F/A' If the `fun F/A` syntax is used multiple times with the same `F/A`, (for examle, `fun foo/2`), there would a wrapper function and fun entry generated for each occurrence. Using the new support in the OTP 23 runtime system, generate a single wrapper function and fun entry for each `F/A`. Since there is only one wrapper function, it can be named based on the name of the function it calls to faciliate debugging, not based on the function that defines the fun. For example, the wrapper function for `fun foo/0` will now be named `-fun.foo/0-'. --- lib/compiler/src/beam_asm.erl | 23 ++++++++++++-- lib/compiler/src/beam_dict.erl | 22 ++++++++++--- lib/compiler/src/compile.erl | 8 +++-- lib/compiler/src/v3_kernel.erl | 63 +++++++++++++++++++++++++++---------- lib/compiler/test/compile_SUITE.erl | 34 +++++++++++++------- lib/compiler/test/test_lib.erl | 1 + lib/tools/test/cprof_SUITE.erl | 12 ++----- 7 files changed, 115 insertions(+), 48 deletions(-) diff --git a/lib/compiler/src/beam_asm.erl b/lib/compiler/src/beam_asm.erl index df09dcb06c..60e19ec596 100644 --- a/lib/compiler/src/beam_asm.erl +++ b/lib/compiler/src/beam_asm.erl @@ -64,11 +64,30 @@ module(Code, ExtraChunks, CompileInfo, CompilerOpts) -> assemble({Mod,Exp0,Attr0,Asm0,NumLabels}, ExtraChunks, CompileInfo, CompilerOpts) -> {1,Dict0} = beam_dict:atom(Mod, beam_dict:new()), {0,Dict1} = beam_dict:fname(atom_to_list(Mod) ++ ".erl", Dict0), + Dict2 = shared_fun_wrappers(CompilerOpts, Dict1), NumFuncs = length(Asm0), {Asm,Attr} = on_load(Asm0, Attr0), Exp = cerl_sets:from_list(Exp0), - {Code,Dict2} = assemble_1(Asm, Exp, Dict1, []), - build_file(Code, Attr, Dict2, NumLabels, NumFuncs, ExtraChunks, CompileInfo, CompilerOpts). + {Code,Dict} = assemble_1(Asm, Exp, Dict2, []), + build_file(Code, Attr, Dict, NumLabels, NumFuncs, + ExtraChunks, CompileInfo, CompilerOpts). + +shared_fun_wrappers(Opts, Dict) -> + case proplists:get_bool(no_shared_fun_wrappers, Opts) of + false -> + %% The compiler in OTP 23 depends on the on the loader + %% using the new indices in funs and being able to have + %% multiple make_fun2 instructions referring to the same + %% fun entry. Artificially set the highest opcode for the + %% module to ensure that it cannot be loaded in OTP 22 + %% and earlier. + Swap = beam_opcodes:opcode(swap, 2), + beam_dict:opcode(Swap, Dict); + true -> + %% Fun wrappers are not shared for compatibility with a + %% previous OTP release. + Dict + end. on_load(Fs0, Attr0) -> case proplists:get_value(on_load, Attr0) of diff --git a/lib/compiler/src/beam_dict.erl b/lib/compiler/src/beam_dict.erl index b2056332e6..4d0cec857d 100644 --- a/lib/compiler/src/beam_dict.erl +++ b/lib/compiler/src/beam_dict.erl @@ -40,6 +40,7 @@ -type lambda_info() :: {label(),{index(),label(),non_neg_integer()}}. -type lambda_tab() :: {non_neg_integer(),[lambda_info()]}. +-type wrapper() :: #{label() => index()}. -record(asm, {atoms = #{} :: atom_tab(), @@ -48,6 +49,7 @@ imports = gb_trees:empty() :: import_tab(), strings = <<>> :: binary(), %String pool lambdas = {0,[]} :: lambda_tab(), + wrappers = #{} :: wrapper(), literals = dict:new() :: literal_tab(), fnames = #{} :: fname_tab(), lines = #{} :: line_tab(), @@ -147,11 +149,21 @@ string(BinString, Dict) when is_binary(BinString) -> -spec lambda(label(), non_neg_integer(), bdict()) -> {non_neg_integer(), bdict()}. -lambda(Lbl, NumFree, #asm{lambdas={OldIndex,Lambdas0}}=Dict) -> - %% Set Index the same as OldIndex. - Index = OldIndex, - Lambdas = [{Lbl,{Index,Lbl,NumFree}}|Lambdas0], - {OldIndex,Dict#asm{lambdas={OldIndex+1,Lambdas}}}. +lambda(Lbl, NumFree, #asm{wrappers=Wrappers0, + lambdas={OldIndex,Lambdas0}}=Dict) -> + case Wrappers0 of + #{Lbl:=Index} -> + %% OTP 23: There old is a fun entry for this wrapper function. + %% Share the fun entry. + {Index,Dict}; + #{} -> + %% Set Index the same as OldIndex. + Index = OldIndex, + Wrappers = Wrappers0#{Lbl=>Index}, + Lambdas = [{Lbl,{Index,Lbl,NumFree}}|Lambdas0], + {OldIndex,Dict#asm{wrappers=Wrappers, + lambdas={OldIndex+1,Lambdas}}} + end. %% Returns the index for a literal (adding it to the literal table if necessary). %% literal(Literal, Dict) -> {Index,Dict'} diff --git a/lib/compiler/src/compile.erl b/lib/compiler/src/compile.erl index 098f82fdc0..42f9e8b902 100644 --- a/lib/compiler/src/compile.erl +++ b/lib/compiler/src/compile.erl @@ -265,9 +265,10 @@ expand_opt(r19, Os) -> expand_opt(r20, Os) -> expand_opt_before_21(Os); expand_opt(r21, Os) -> - [no_swap, no_put_tuple2 | expand_opt(no_bsm3, Os)]; + [no_shared_fun_wrappers, + no_swap, no_put_tuple2 | expand_opt(no_bsm3, Os)]; expand_opt(r22, Os) -> - [no_swap | Os]; + [no_shared_fun_wrappers, no_swap | Os]; expand_opt({debug_info_key,_}=O, Os) -> [encrypt_debug_info,O|Os]; expand_opt(no_type_opt, Os) -> @@ -277,7 +278,8 @@ expand_opt(no_type_opt, Os) -> expand_opt(O, Os) -> [O|Os]. expand_opt_before_21(Os) -> - [no_swap, no_put_tuple2, no_get_hd_tl, no_ssa_opt_record, + [no_shared_fun_wrappers, no_swap, + no_put_tuple2, no_get_hd_tl, no_ssa_opt_record, no_utf8_atoms | expand_opt(no_bsm3, Os)]. %% format_error(ErrorDescriptor) -> string() diff --git a/lib/compiler/src/v3_kernel.erl b/lib/compiler/src/v3_kernel.erl index e2b8787224..6fd1790c1a 100644 --- a/lib/compiler/src/v3_kernel.erl +++ b/lib/compiler/src/v3_kernel.erl @@ -81,8 +81,11 @@ -export([module/2,format_error/1]). --import(lists, [map/2,foldl/3,foldr/3,mapfoldl/3,splitwith/2,member/2, - keyfind/3,partition/2,droplast/1,last/1,sort/1,reverse/1]). +-import(lists, [droplast/1,flatten/1,foldl/3,foldr/3, + map/2,mapfoldl/3,member/2, + keyfind/3,keyreplace/4, + last/1,partition/2,reverse/1, + splitwith/2,sort/1]). -import(ordsets, [add_element/2,del_element/2,union/2,union/1,subtract/2]). -import(cerl, [c_tuple/1]). @@ -120,15 +123,19 @@ copy_anno(Kdst, Ksrc) -> funs=[], %Fun functions free=#{}, %Free variables ws=[] :: [warning()], %Warnings. - guard_refc=0}). %> 0 means in guard + guard_refc=0, %> 0 means in guard + no_shared_fun_wrappers=false :: boolean() + }). -spec module(cerl:c_module(), [compile:option()]) -> {'ok', #k_mdef{}, [warning()]}. -module(#c_module{anno=A,name=M,exports=Es,attrs=As,defs=Fs}, _Options) -> +module(#c_module{anno=A,name=M,exports=Es,attrs=As,defs=Fs}, Options) -> Kas = attributes(As), Kes = map(fun (#c_var{name={_,_}=Fname}) -> Fname end, Es), - St0 = #kern{}, + NoSharedFunWrappers = proplists:get_bool(no_shared_fun_wrappers, + Options), + St0 = #kern{no_shared_fun_wrappers=NoSharedFunWrappers}, {Kfs,St} = mapfoldl(fun function/2, St0, Fs), {ok,#k_mdef{anno=A,name=M#c_literal.val,exports=Kes,attributes=Kas, body=Kfs ++ St#kern.funs},lists:sort(St#kern.ws)}. @@ -716,16 +723,27 @@ gexpr_test_add(Ke, St0) -> %% expr(Cexpr, Sub, State) -> {Kexpr,[PreKexpr],State}. %% Convert a Core expression, flattening it at the same time. -expr(#c_var{anno=A,name={_Name,Arity}}=Fname, Sub, St) -> - %% A local in an expression. - %% For now, these are wrapped into a fun by reverse - %% eta-conversion, but really, there should be exactly one - %% such "lambda function" for each escaping local name, - %% instead of one for each occurrence as done now. +expr(#c_var{anno=A0,name={Name,Arity}}=Fname, Sub, St) -> Vs = [#c_var{name=list_to_atom("V" ++ integer_to_list(V))} || - V <- integers(1, Arity)], - Fun = #c_fun{anno=A,vars=Vs,body=#c_apply{anno=A,op=Fname,args=Vs}}, - expr(Fun, Sub, St); + V <- integers(1, Arity)], + case St#kern.no_shared_fun_wrappers of + false -> + %% Generate a (possibly shared) wrapper function for calling + %% this function. + Wrapper0 = ["-fun.",atom_to_list(Name),"/",integer_to_list(Arity),"-"], + Wrapper = list_to_atom(flatten(Wrapper0)), + Id = {id,{0,0,Wrapper}}, + A = keyreplace(id, 1, A0, Id), + Fun = #c_fun{anno=A,vars=Vs,body=#c_apply{anno=A,op=Fname,args=Vs}}, + expr(Fun, Sub, St); + true -> + %% For backward compatibility with OTP 22 and earlier, + %% use the pre-generated name for the fun wrapper. + %% There will be one wrapper function for each occurrence + %% of `fun F/A`. + Fun = #c_fun{anno=A0,vars=Vs,body=#c_apply{anno=A0,op=Fname,args=Vs}}, + expr(Fun, Sub, St) + end; expr(#c_var{anno=A,name=V}, Sub, St) -> {#k_var{anno=A,name=get_vsub(V, Sub)},[],St}; expr(#c_literal{anno=A,val=V}, _Sub, St) -> @@ -2446,8 +2464,21 @@ uexpr(Lit, {break,Rs0}, St0) -> {#k_put{anno=#k{us=Used,ns=lit_list_vars(Rs),a=get_kanno(Lit)}, arg=Lit,ret=Rs},Used,St1}. -add_local_function(_, #kern{funs=ignore}=St) -> St; -add_local_function(F, #kern{funs=Funs}=St) -> St#kern{funs=[F|Funs]}. +add_local_function(_, #kern{funs=ignore}=St) -> + St; +add_local_function(#k_fdef{func=Name,arity=Arity}=F, #kern{funs=Funs}=St) -> + case is_defined(Name, Arity, Funs) of + false -> + St#kern{funs=[F|Funs]}; + true -> + St + end. + +is_defined(Name, Arity, [#k_fdef{func=Name,arity=Arity}|_]) -> + true; +is_defined(Name, Arity, [#k_fdef{}|T]) -> + is_defined(Name, Arity, T); +is_defined(_, _, []) -> false. %% Make a #k_fdef{}, making sure that the body is always a #k_match{}. make_fdef(Anno, Name, Arity, Vs, #k_match{}=Body) -> diff --git a/lib/compiler/test/compile_SUITE.erl b/lib/compiler/test/compile_SUITE.erl index 7e9e641478..453debc0c1 100644 --- a/lib/compiler/test/compile_SUITE.erl +++ b/lib/compiler/test/compile_SUITE.erl @@ -1382,27 +1382,33 @@ env_compiler_options(_Config) -> bc_options(Config) -> DataDir = proplists:get_value(data_dir, Config), - L = [{101, small_float, [no_get_hd_tl,no_line_info]}, - {103, big, [no_put_tuple2,no_get_hd_tl,no_ssa_opt_record, + L = [{101, small_float, [no_shared_fun_wrappers, + no_get_hd_tl,no_line_info]}, + {103, big, [no_shared_fun_wrappers, + no_put_tuple2,no_get_hd_tl,no_ssa_opt_record, no_line_info,no_stack_trimming]}, - {125, small_float, [no_get_hd_tl,no_line_info,no_ssa_opt_float]}, + {125, small_float, [no_shared_fun_wrappers,no_get_hd_tl, + no_line_info, + no_ssa_opt_float]}, - {132, small, [no_put_tuple2,no_get_hd_tl,no_ssa_opt_record, + {132, small, [no_shared_fun_wrappers, + no_put_tuple2,no_get_hd_tl,no_ssa_opt_record, no_ssa_opt_float,no_line_info,no_bsm3]}, + {136, big, [no_shared_fun_wrappers,no_put_tuple2,no_get_hd_tl, + no_ssa_opt_record,no_line_info]}, + {153, small, [r20]}, {153, small, [r21]}, - {136, big, [no_put_tuple2,no_get_hd_tl, - no_ssa_opt_record,no_line_info]}, - - {153, big, [no_put_tuple2,no_get_hd_tl, no_ssa_opt_record]}, + {153, big, [no_shared_fun_wrappers, + no_put_tuple2,no_get_hd_tl, no_ssa_opt_record]}, {153, big, [r16]}, {153, big, [r17]}, {153, big, [r18]}, {153, big, [r19]}, {153, small_float, [r16]}, - {153, small_float, []}, + {153, small_float, [no_shared_fun_wrappers]}, {158, small_maps, [r17]}, {158, small_maps, [r18]}, @@ -1412,11 +1418,15 @@ bc_options(Config) -> {164, small_maps, [r22]}, {164, big, [r22]}, - {164, small_maps, []}, - {164, big, []}, + {164, small_maps, [no_shared_fun_wrappers]}, + {164, big, [no_shared_fun_wrappers]}, {168, small, [r22]}, - {168, small, []} + {168, small, [no_shared_fun_wrappers]}, + + {169, small_maps, []}, + {169, big, []}, + {169, small, []} ], Test = fun({Expected,Mod,Options}) -> diff --git a/lib/compiler/test/test_lib.erl b/lib/compiler/test/test_lib.erl index 98210a351c..a468482acb 100644 --- a/lib/compiler/test/test_lib.erl +++ b/lib/compiler/test/test_lib.erl @@ -79,6 +79,7 @@ opt_opts(Mod) -> (no_put_tuple2) -> true; (no_recv_opt) -> true; (no_share_opt) -> true; + (no_shared_fun_wrappers) -> true; (no_ssa_float) -> true; (no_ssa_opt) -> true; (no_stack_trimming) -> true; diff --git a/lib/tools/test/cprof_SUITE.erl b/lib/tools/test/cprof_SUITE.erl index 9cbc27fb17..39239a66a9 100644 --- a/lib/tools/test/cprof_SUITE.erl +++ b/lib/tools/test/cprof_SUITE.erl @@ -211,16 +211,12 @@ on_load_test(Config) -> Lr = seq_r(1, M, fun succ/1), N2 = cprof:pause(), {Module,0,[]} = cprof:analyse(Module), - M_1 = M - 1, M4__4 = M*4 - 4, M10_7 = M*10 - 7, {?MODULE,M10_7,[{{?MODULE,succ,1},M4__4}, + {{?MODULE,'-fun.succ/1-',1},M4__4}, {{?MODULE,seq_r,4},M}, {{?MODULE,seq,3},M}, - {{?MODULE,'-on_load_test/1-fun-5-',1},M_1}, - {{?MODULE,'-on_load_test/1-fun-4-',1},M_1}, - {{?MODULE,'-on_load_test/1-fun-3-',1},M_1}, - {{?MODULE,'-on_load_test/1-fun-2-',1},M_1}, {{?MODULE,seq_r,3},1}]} = cprof:analyse(?MODULE), N2 = cprof:stop(), @@ -246,18 +242,14 @@ modules_test(Config) -> Lr = seq_r(1, M, fun succ/1), N = cprof:pause(), Lr = lists:reverse(L), - M_1 = M - 1, M4_4 = M*4 - 4, M10_7 = M*10 - 7, M2__1 = M*2 + 1, {Tot,ModList} = cprof:analyse(), {value,{?MODULE,M10_7,[{{?MODULE,succ,1},M4_4}, + {{?MODULE,'-fun.succ/1-',1},M4_4}, {{?MODULE,seq_r,4},M}, {{?MODULE,seq,3},M}, - {{?MODULE,'-modules_test/1-fun-3-',1},M_1}, - {{?MODULE,'-modules_test/1-fun-2-',1},M_1}, - {{?MODULE,'-modules_test/1-fun-1-',1},M_1}, - {{?MODULE,'-modules_test/1-fun-0-',1},M_1}, {{?MODULE,seq_r,3},1}]}} = lists:keysearch(?MODULE, 1, ModList), {value,{Module,M2__1,[{{Module,seq_r,4},M}, -- cgit v1.2.3