From a2c88ff1875a2039c987c1099e6d911f1b6dfce6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 18 Mar 2010 11:18:11 +0100 Subject: sys_core_inline: Don't generated multiple compiler_generated annos Multiple compiler_generated annotations are harmless, but makes listing files harder to read during debugging. --- lib/compiler/src/sys_core_inline.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/sys_core_inline.erl b/lib/compiler/src/sys_core_inline.erl index c8d75b80c6..06696e5950 100644 --- a/lib/compiler/src/sys_core_inline.erl +++ b/lib/compiler/src/sys_core_inline.erl @@ -201,7 +201,7 @@ kill_id_anns(Body) -> (Expr) -> %% Mark everything as compiler generated to suppress %% bogus warnings. - A = [compiler_generated|core_lib:get_anno(Expr)], + A = compiler_generated(core_lib:get_anno(Expr)), core_lib:set_anno(Expr, A) end, Body). @@ -210,3 +210,8 @@ kill_id_anns_1([{'id',_}|As]) -> kill_id_anns_1([A|As]) -> [A|kill_id_anns_1(As)]; kill_id_anns_1([]) -> []. + +compiler_generated([compiler_generated|_]=Anno) -> + Anno; +compiler_generated(Anno) -> + [compiler_generated|Anno -- [compiler_generated]]. -- cgit v1.2.3 From e2074da57e6dd9b68a39ae71771b28c8f704196f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Wed, 17 Mar 2010 16:04:52 +0100 Subject: compiler: Fix binary matching bug in the inliner The inliner incorrectly assumes that a literal cannot match a binary in code such as: t() -> bc(<<"string">>). bc(<>) -> [A|bc(T)]; bc(<<>>) -> []. The bug was introduced when binary literals were introduced. --- lib/compiler/src/cerl_clauses.erl | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/cerl_clauses.erl b/lib/compiler/src/cerl_clauses.erl index 5f111a5e05..bb0f998931 100644 --- a/lib/compiler/src/cerl_clauses.erl +++ b/lib/compiler/src/cerl_clauses.erl @@ -338,10 +338,19 @@ match(P, E, Bs) -> if E =:= any -> {false, Bs}; true -> - case is_data(E) of - true -> + case type(E) of + literal -> + case is_bitstring(concrete(E)) of + false -> + none; + true -> + {false, Bs} + end; + cons -> none; - false -> + tuple -> + none; + _ -> {false, Bs} end end; -- cgit v1.2.3 From 57cb16f84fd21443d3ad9951473f1e0960c6a26e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Wed, 17 Mar 2010 16:09:48 +0100 Subject: compiler: Suppress bs_context_to_binary/1 for a literal operand The inliner can cause illegal uses of the bs_context_to_binary/1 instruction, such as: bs_context_to_binary {literal,"string"} or bs_context_to_binary {integer,1} Remove the bs_context_to_binary/1 instruction if the operand is not a register (it is clearly not needed). --- lib/compiler/src/v3_codegen.erl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl index 1f05b4f6b4..948937c438 100644 --- a/lib/compiler/src/v3_codegen.erl +++ b/lib/compiler/src/v3_codegen.erl @@ -1190,7 +1190,12 @@ trap_bif(_, _, _) -> false. bif_cg(bs_context_to_binary=Instr, [Src0], [], Le, Vdb, Bef, St0) -> [Src] = cg_reg_args([Src0], Bef), - {[{Instr,Src}],clear_dead(Bef, Le#l.i, Vdb), St0}; + case is_register(Src) of + false -> + {[],clear_dead(Bef, Le#l.i, Vdb), St0}; + true -> + {[{Instr,Src}],clear_dead(Bef, Le#l.i, Vdb), St0} + end; bif_cg(dsetelement, [Index0,Tuple0,New0], _Rs, Le, Vdb, Bef, St0) -> [New,Tuple,{integer,Index1}] = cg_reg_args([New0,Tuple0,Index0], Bef), Index = Index1-1, @@ -2019,6 +2024,10 @@ fetch_stack(V, [_|Stk], I) -> fetch_stack(V, Stk, I+1). on_stack(V, Stk) -> keymember(V, 1, Stk). +is_register({x,_}) -> true; +is_register({yy,_}) -> true; +is_register(_) -> false. + %% put_catch(CatchTag, Stack) -> Stack' %% drop_catch(CatchTag, Stack) -> Stack' %% Special interface for putting and removing catch tags, to ensure that -- cgit v1.2.3 From 0320836d1b58438ef4467832b00306672d22f8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 18 Mar 2010 08:45:03 +0100 Subject: Consistently rewrite an inlined function_clause exception to case_clause A function_clause exception is generated by jumping to a func_info/3 instruction at the beginning of the function. The x registers are assumed to contain the arguments for the function. That means that a func_info/3 instruction copied from another function (or even from the same function if not at the top level) will not work, so it must be replaced with an instruction that generates a case_clause exception. In Core Erlang, a func_info/3 instruction is represented as a the primop match_fail({function_clause,Arg1,...ArgN}). The current mechanism that is supposed to replace the primop match_fail(function_clause) with match_fail(case_clause) will fail to do that in the following circumstances: 1. If the inliner has inlined a function into itself. Fix that by having the inliner clear the function_name annotations on all match_fail primops in functions that are inlined. (To simplify doing that, the annotation is now on the primop node itself and not on the 'function_clause' atom inside it.) 2. If the inliner has rewritten the tuple node in the primop node to a literal (when inlining a function call with literal arguments), v3_kernel would not recognize the match_fail(function_clause) primop and would not rewrite it. Fix it by making v3_kernel smarter. Also simplify the "old" inliner (sys_core_inline) to only clear the function_name annotations instead of rewriting function_clause execptions to case_clause execptions itself. --- lib/compiler/src/cerl_inline.erl | 32 ++++++++++++++--- lib/compiler/src/sys_core_inline.erl | 12 +++---- lib/compiler/src/v3_core.erl | 22 +++++++----- lib/compiler/src/v3_kernel.erl | 66 ++++++++++++++++++++++++------------ 4 files changed, 88 insertions(+), 44 deletions(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/cerl_inline.erl b/lib/compiler/src/cerl_inline.erl index 191efa3032..f4eaa17e72 100644 --- a/lib/compiler/src/cerl_inline.erl +++ b/lib/compiler/src/cerl_inline.erl @@ -1429,17 +1429,26 @@ inline(E, #app{opnds = Opnds, ctxt = Ctxt, loc = L}, Ren, Env, S) -> {E, S}; true -> %% Create local bindings for the parameters to their - %% respective operand structures from the app-structure, and - %% visit the body in the context saved in the structure. + %% respective operand structures from the app-structure. {Rs, Ren1, Env1, S1} = bind_locals(Vs, Opnds, Ren, Env, S), - {E1, S2} = i(fun_body(E), Ctxt, Ren1, Env1, S1), + + %% function_clause exceptions that have been inlined + %% into another function (or even into the same function) + %% will not work properly. The v3_kernel pass will + %% take care of it, but we will need to help it by + %% removing any function_name annotations on match_fail + %% primops that we inline. + E1 = kill_function_name_anns(fun_body(E)), + + %% Visit the body in the context saved in the structure. + {E2, S2} = i(E1, Ctxt, Ren1, Env1, S1), %% Create necessary bindings and/or set flags. - {E2, S3} = make_let_bindings(Rs, E1, S2), + {E3, S3} = make_let_bindings(Rs, E2, S2), %% Lastly, flag the application as inlined, since the inlining %% attempt was not aborted before we reached this point. - {E2, st__set_app_inlined(L, S3)} + {E3, st__set_app_inlined(L, S3)} end. %% For the (possibly renamed) argument variables to an inlined call, @@ -2370,6 +2379,19 @@ kill_id_anns([A | As]) -> kill_id_anns([]) -> []. +kill_function_name_anns(Body) -> + F = fun(P) -> + case type(P) of + primop -> + Ann = get_ann(P), + Ann1 = lists:keydelete(function_name, 1, Ann), + set_ann(P, Ann1); + _ -> + P + end + end, + cerl_trees:map(F, Body). + %% ===================================================================== %% General utilities diff --git a/lib/compiler/src/sys_core_inline.erl b/lib/compiler/src/sys_core_inline.erl index 06696e5950..c644b9e015 100644 --- a/lib/compiler/src/sys_core_inline.erl +++ b/lib/compiler/src/sys_core_inline.erl @@ -41,11 +41,9 @@ -module(sys_core_inline). -%%-compile({inline,{match_fail_fun,0}}). - -export([module/2]). --import(lists, [member/2,map/2,foldl/3,mapfoldl/3]). +-import(lists, [member/2,map/2,foldl/3,mapfoldl/3,keydelete/3]). -include("core_parse.hrl"). @@ -178,11 +176,9 @@ weight_func(_Core, Acc) -> Acc + 1. %% function_clause match_fail (if they have one). match_fail_fun() -> - fun (#c_primop{name=#c_literal{val=match_fail}, - args=[#c_tuple{es=[#c_literal{val=function_clause}|As]}]}=P) -> - Fail = #c_tuple{es=[#c_literal{val=case_clause}, - #c_tuple{es=As}]}, - P#c_primop{args=[Fail]}; + fun (#c_primop{anno=Anno0,name=#c_literal{val=match_fail}}=P) -> + Anno = keydelete(function_name, 1, Anno0), + P#c_primop{anno=Anno}; (Other) -> Other end. diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl index 8b04969b05..4ac2196d79 100644 --- a/lib/compiler/src/v3_core.erl +++ b/lib/compiler/src/v3_core.erl @@ -1542,17 +1542,21 @@ new_vars_1(N, Anno, St0, Vs) when N > 0 -> new_vars_1(0, _, St, Vs) -> {Vs,St}. function_clause(Ps, Name) -> - fail_clause(Ps, c_tuple([#c_literal{anno=[{name,Name}], - val=function_clause}|Ps])). -function_clause(Ps, Anno, Name) -> - fail_clause(Ps, ann_c_tuple(Anno, - [#c_literal{anno=[{name,Name}], - val=function_clause}|Ps])). - -fail_clause(Pats, A) -> + function_clause(Ps, [], Name). + +function_clause(Ps, LineAnno, Name) -> + FcAnno = [{function_name,Name}], + fail_clause(Ps, FcAnno, + ann_c_tuple(LineAnno, [#c_literal{val=function_clause}|Ps])). + +fail_clause(Pats, Arg) -> + fail_clause(Pats, [], Arg). + +fail_clause(Pats, Anno, Arg) -> #iclause{anno=#a{anno=[compiler_generated]}, pats=Pats,guard=[], - body=[#iprimop{anno=#a{},name=#c_literal{val=match_fail},args=[A]}]}. + body=[#iprimop{anno=#a{anno=Anno},name=#c_literal{val=match_fail}, + args=[Arg]}]}. ubody(B, St) -> uexpr(B, [], St). diff --git a/lib/compiler/src/v3_kernel.erl b/lib/compiler/src/v3_kernel.erl index f1215e8a35..974c64a8bc 100644 --- a/lib/compiler/src/v3_kernel.erl +++ b/lib/compiler/src/v3_kernel.erl @@ -80,7 +80,8 @@ -export([module/2,format_error/1]). --import(lists, [map/2,foldl/3,foldr/3,mapfoldl/3,splitwith/2,member/2,keymember/3]). +-import(lists, [map/2,foldl/3,foldr/3,mapfoldl/3,splitwith/2,member/2, + keymember/3,keyfind/3]). -import(ordsets, [add_element/2,del_element/2,union/2,union/1,subtract/2]). -compile({nowarn_deprecated_function, {erlang,hash,2}}). @@ -408,7 +409,7 @@ expr(#c_call{anno=A,module=M0,name=F0,args=Cargs}, Sub, St0) -> {Call,Ap,St} end; expr(#c_primop{anno=A,name=#c_literal{val=match_fail},args=Cargs0}, Sub, St0) -> - Cargs = translate_match_fail(Cargs0, Sub, St0), + Cargs = translate_match_fail(Cargs0, Sub, A, St0), %% This special case will disappear. {Kargs,Ap,St} = atomic_list(Cargs, Sub, St0), Ar = length(Cargs), @@ -435,32 +436,53 @@ expr(#c_catch{anno=A,body=Cb}, Sub, St0) -> %% Handle internal expressions. expr(#ireceive_accept{anno=A}, _Sub, St) -> {#k_receive_accept{anno=A},[],St}. -%% Translate a function_clause to case_clause if it has been moved into -%% another function. -translate_match_fail([#c_tuple{es=[#c_literal{anno=A0, - val=function_clause}|As]}]=Args, - Sub, - #kern{ff=FF}) -> - A = case A0 of - [{name,{Func0,Arity0}}] -> - [{name,{get_fsub(Func0, Arity0, Sub),Arity0}}]; - _ -> - A0 - end, - case {A,FF} of - {[{name,Same}],Same} -> +%% Translate a function_clause exception to a case_clause exception if +%% it has been moved into another function. (A function_clause exception +%% will not work correctly if it is moved into another function, or +%% even if it is invoked not from the top level in the correct function.) +translate_match_fail(Args, Sub, Anno, St) -> + case Args of + [#c_tuple{es=[#c_literal{val=function_clause}|As]}] -> + translate_match_fail_1(Anno, Args, As, Sub, St); + [#c_literal{val=Tuple}] when is_tuple(Tuple) -> + %% The inliner may have created a literal out of + %% the original #c_tuple{}. + case tuple_to_list(Tuple) of + [function_clause|As0] -> + As = [#c_literal{val=E} || E <- As0], + translate_match_fail_1(Anno, Args, As, Sub, St); + _ -> + Args + end; + _ -> + %% Not a function_clause exception. + Args + end. + +translate_match_fail_1(Anno, Args, As, Sub, #kern{ff=FF}) -> + AnnoFunc = case keyfind(function_name, 1, Anno) of + false -> + none; %Force rewrite. + {function_name,{Name,Arity}} -> + {get_fsub(Name, Arity, Sub),Arity} + end, + case {AnnoFunc,FF} of + {Same,Same} -> %% Still in the correct function. Args; - {[{name,{F,_}}],F} -> + {{F,_},F} -> %% Still in the correct function. Args; _ -> - %% Inlining has probably moved the function_clause into another - %% function (where it will not work correctly). - %% Rewrite to a case_clause. + %% Wrong function or no function_name annotation. + %% + %% The inliner has copied the match_fail(function_clause) + %% primop from another function (or from another instance of + %% the current function). match_fail(function_clause) will + %% only work at the top level of the function it was originally + %% defined in, so we will need to rewrite it to a case_clause. [#c_tuple{es=[#c_literal{val=case_clause},#c_tuple{es=As}]}] - end; -translate_match_fail(Args, _, _) -> Args. + end. %% call_type(Module, Function, Arity) -> call | bif | apply | error. %% Classify the call. -- cgit v1.2.3