sys_core_fold: Optimize let statements more aggressively

I originally decided that in 'value' context, rewriting a let statement
where the variables were not in the body to a sequence was not worth
it, because the variables would be unused in only one let in a
thousand lets (roughly).

I have reconsidered.

The main reason is that if we do the rewrite, core_lib:is_var_used/2
will be used much more frequently, which will help us to find bugs
in it sooner.

Another reason is that the way letify/2 is currently implemented
with its own calls to core_lib:is_var_used/2 is only safe as long
as all the bindings are independent of each other. We could make
letify/2 smarter, but if we introduce this new optimization there
is no need.

Measuring compilation speed, I have not seen any significant slowdown.
It seems that although core_lib:is_var_used/2 is called much more
frequently, most calls will be fast because is_var_used/2 will quickly
find a use of the variable.

Also add a test case to cover a line opt_guard_try/1 that was
no longer covered.

5 files changed, 31 insertions, 19 deletions

diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl
index eeb3535936..72509947d6 100644
--- a/lib/compiler/src/sys_core_fold.erl
+++ b/lib/compiler/src/sys_core_fold.erl
@@ -2246,18 +2246,11 @@ make_var_name() ->
     list_to_atom("fol"++integer_to_list(N)).
 
 letify(Bs, Body) ->
+    Ann = cerl:get_ann(Body),
     foldr(fun({V,Val}, B) ->
-		  letify(V, Val, B)
+		  cerl:ann_c_let(Ann, [V], Val, B)
 	  end, Body, Bs).
 
-letify(#c_var{name=Vname}=Var, Val, Body) ->
-    case core_lib:is_var_used(Vname, Body) of
-	true ->
-	    A = element(2, Body),
-	    #c_let{anno=A,vars=[Var],arg=Val,body=Body};
-	false -> Body
-    end.
-
 %% opt_case_in_let(LetExpr) -> LetExpr'
 
 opt_case_in_let(#c_let{vars=Vs,arg=Arg,body=B}=Let, Sub) ->
@@ -2622,7 +2615,7 @@ opt_simple_let_2(Let0, Vs0, Arg0, Body0, effect, Sub) ->
 		    opt_case_in_let_arg(opt_case_in_let(Let, Sub), effect, Sub)
 	    end
     end;
-opt_simple_let_2(Let, Vs0, Arg0, Body, value, Sub) ->
+opt_simple_let_2(Let0, Vs0, Arg0, Body, value, Sub) ->
     case {Vs0,Arg0,Body} of
 	{[#c_var{name=N1}],Arg,#c_var{name=N2}} ->
 	    case N1 =:= N2 of
@@ -2641,9 +2634,17 @@ opt_simple_let_2(Let, Vs0, Arg0, Body, value, Sub) ->
 	    %% can be evaluated in effect context to simplify it.
 	    expr(#c_seq{arg=Arg,body=Body}, value, sub_new_preserve_types(Sub));
 	{Vs,Arg,Body} ->
-	    opt_case_in_let_arg(
-	      opt_case_in_let(Let#c_let{vars=Vs,arg=Arg,body=Body}, Sub),
-	      value, Sub)
+	    %% If none of the variables are used in the body, we can rewrite the
+	    %% let to a sequence:
+	    %%   let <Var> = Arg in BodyWithoutVar  ==> seq Arg BodyWithoutVar
+	    case is_any_var_used(Vs, Body) of
+		false ->
+		    expr(#c_seq{arg=Arg,body=Body}, value,
+			 sub_new_preserve_types(Sub));
+		true ->
+		    Let = Let0#c_let{vars=Vs,arg=Arg,body=Body},
+		    opt_case_in_let_arg(opt_case_in_let(Let, Sub), value, Sub)
+	    end
     end.
 
 move_case_into_arg(#c_case{arg=#c_let{vars=OuterVars0,arg=OuterArg,
diff --git a/lib/compiler/test/compilation_SUITE.erl b/lib/compiler/test/compilation_SUITE.erl
index 8711f35e8e..296774e083 100644
--- a/lib/compiler/test/compilation_SUITE.erl
+++ b/lib/compiler/test/compilation_SUITE.erl
@@ -611,12 +611,10 @@ otp_7345(Config) when is_list(Config) ->
 
 otp_7345(ObjRef, _RdEnv, Args) ->
     Cid = ObjRef#contextId.cid,
-    _DpRef =
-	#dpRef{cid = Cid,
+    _ =	#dpRef{cid = Cid,
 		     ms_device_context_id = cid_id,
 		     tlli = #ptmsi{value = 0}},
-    _QosProfile =
-	#qosProfileBssgp{peak_bit_rate_msb = 0,
+    _ =	#qosProfileBssgp{peak_bit_rate_msb = 0,
 			 peak_bit_rate_lsb = 80,
 			 t_a_precedence = 49},
     [Cpdu|_] = Args,
diff --git a/lib/compiler/test/core_fold_SUITE.erl b/lib/compiler/test/core_fold_SUITE.erl
index ce1aea5de0..bca8afb1b0 100644
--- a/lib/compiler/test/core_fold_SUITE.erl
+++ b/lib/compiler/test/core_fold_SUITE.erl
@@ -242,6 +242,8 @@ do_guard_try_catch(K, V) ->
 	    false
     end.
 
+-record(cover_opt_guard_try, {list=[]}).
+
 coverage(Config) when is_list(Config) ->
     ?line {'EXIT',{{case_clause,{a,b,c}},_}} =
 	(catch cover_will_match_list_type({a,b,c})),
@@ -251,6 +253,9 @@ coverage(Config) when is_list(Config) ->
     ?line error = cover_will_match_lit_list(),
     {ok,[a]} = cover_is_safe_bool_expr(a),
 
+    ok = cover_opt_guard_try(#cover_opt_guard_try{list=[a]}),
+    error = cover_opt_guard_try(#cover_opt_guard_try{list=[]}),
+
     %% Make sure that we don't attempt to make literals
     %% out of pids. (Putting a pid into a #c_literal{}
     %% would crash later compiler passes.)
@@ -304,6 +309,14 @@ cover_is_safe_bool_expr(X) ->
 	    false
     end.
 
+cover_opt_guard_try(Msg) ->
+    if
+	length(Msg#cover_opt_guard_try.list) =/= 1 ->
+	    error;
+	true ->
+	    ok
+    end.
+
 bsm_an_inlined(<<_:8>>, _) -> ok;
 bsm_an_inlined(_, _) -> error.
 
diff --git a/lib/compiler/test/trycatch_SUITE.erl b/lib/compiler/test/trycatch_SUITE.erl
index b490257a6e..8ab618bb01 100644
--- a/lib/compiler/test/trycatch_SUITE.erl
+++ b/lib/compiler/test/trycatch_SUITE.erl
@@ -790,7 +790,6 @@ nested_after_1({X1,C1,V1},
 
 
 nested_horrid(Config) when is_list(Config) ->
-    _V = {make_ref(),nested_horrid,4.711},
     {[true,true],{[true,1.0],1.0}} =
 	nested_horrid_1({true,void,void}, 1.0),
     ok.
diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl
index d59c89b264..6663985ad7 100644
--- a/lib/compiler/test/warnings_SUITE.erl
+++ b/lib/compiler/test/warnings_SUITE.erl
@@ -281,11 +281,12 @@ bad_arith(Config) when is_list(Config) ->
 	     {3,sys_core_fold,{eval_failure,badarith}},
 	     {9,sys_core_fold,nomatch_guard},
 	     {9,sys_core_fold,{eval_failure,badarith}},
+	     {9,sys_core_fold,{no_effect,{erlang,is_integer,1}}},
 	     {10,sys_core_fold,nomatch_guard},
 	     {10,sys_core_fold,{eval_failure,badarith}},
 	     {15,sys_core_fold,{eval_failure,badarith}}
 	    ] }}],
-    ?line [] = run(Config, Ts),
+    [] = run(Config, Ts),
     ok.
 
 bool_cases(Config) when is_list(Config) ->