Move select_val optimization from beam_clean to beam_peep

There is an optimization in beam_clean that will remove values
having the same label as the failure label in a select_val
instruction. Conceptually, this optimization is in the wrong
module since ideally beam_clean is a mandatory pass that should
not do optimizations. Furthermore, this part of beam_clean is
called three times (from beam_dead, beam_peep, and as a compiler
pass from the 'compile' module), but it only does useful one of
the times it is called.

Therefore, move this optimization to the beam_peep pass.

The same optimization is done in beam_dead, but unfortunately it
misses some opportunities for optimization because the code sharing
optimization in beam_jump (share/1) runs after beam_dead. It would
be more satisfactory to have this optimization only in beam_dead,
but it turned out not to be trivial. If we try to run
beam_jump:share/1 before beam_dead, some optimizations will no
longer work in beam_dead because fallthroughs have been eliminated.

For the moment, the possible solutions to this problem seems to
involve more work and more complicated code than the gain from
eliminating the duplicated optimization would gain.

2 files changed, 20 insertions, 16 deletions

diff --git a/lib/compiler/src/beam_clean.erl b/lib/compiler/src/beam_clean.erl
index 7d66985791..d9108c383d 100644
--- a/lib/compiler/src/beam_clean.erl
+++ b/lib/compiler/src/beam_clean.erl
@@ -190,17 +190,11 @@ replace([{test,Test,{f,Lbl},Ops}|Is], Acc, D) ->
 replace([{test,Test,{f,Lbl},Live,Ops,Dst}|Is], Acc, D) ->
     replace(Is, [{test,Test,{f,label(Lbl, D)},Live,Ops,Dst}|Acc], D);
 replace([{select,I,R,{f,Fail0},Vls0}|Is], Acc, D) ->
-    Vls1 = map(fun ({f,L}) -> {f,label(L, D)};
-		   (Other) -> Other end, Vls0),
+    Vls = map(fun ({f,L}) -> {f,label(L, D)};
+		   (Other) -> Other
+	      end, Vls0),
     Fail = label(Fail0, D),
-    case redundant_values(Vls1, Fail, []) of
-	[] ->
-	    %% Oops, no choices left. The loader will not accept that.
-	    %% Convert to a plain jump.
-	    replace(Is, [{jump,{f,Fail}}|Acc], D);
-	Vls ->
-	    replace(Is, [{select,I,R,{f,Fail},Vls}|Acc], D)
-    end;
+    replace(Is, [{select,I,R,{f,Fail},Vls}|Acc], D);
 replace([{'try',R,{f,Lbl}}|Is], Acc, D) ->
     replace(Is, [{'try',R,{f,label(Lbl, D)}}|Acc], D);
 replace([{'catch',R,{f,Lbl}}|Is], Acc, D) ->
@@ -241,12 +235,6 @@ label(Old, D) ->
 	{value,Val} -> Val;
 	none -> throw({error,{undefined_label,Old}})
     end.
-	    
-redundant_values([_,{f,Fail}|Vls], Fail, Acc) ->
-    redundant_values(Vls, Fail, Acc);
-redundant_values([Val,Lbl|Vls], Fail, Acc) ->
-    redundant_values(Vls, Fail, [Lbl,Val|Acc]);
-redundant_values([], _, Acc) -> reverse(Acc).
 
 %%%
 %%% Final fixup of bs_start_match2/5,bs_save2/bs_restore2 instructions for
diff --git a/lib/compiler/src/beam_peep.erl b/lib/compiler/src/beam_peep.erl
index 75be86b83f..0c1abfe6a0 100644
--- a/lib/compiler/src/beam_peep.erl
+++ b/lib/compiler/src/beam_peep.erl
@@ -83,6 +83,16 @@ peep([{gc_bif,_,_,_,_,Dst}=I|Is], SeenTests0, Acc) ->
     %% Kill all remembered tests that depend on the destination register.
     SeenTests = kill_seen(Dst, SeenTests0),
     peep(Is, SeenTests, [I|Acc]);
+peep([{select,Op,R,F,Vls0}|Is], _, Acc) ->
+    case prune_redundant_values(Vls0, F) of
+	[] ->
+	    %% No values left. Must convert to plain jump.
+	    I = {jump,F},
+	    peep(Is, gb_sets:empty(), [I|Acc]);
+	[_|_]=Vls ->
+	    I = {select,Op,R,F,Vls},
+	    peep(Is, gb_sets:empty(), [I|Acc])
+    end;
 peep([{test,Op,_,Ops}=I|Is], SeenTests0, Acc) ->
     case beam_utils:is_pure_test(I) of
 	false ->
@@ -127,3 +137,9 @@ kill_seen_1([{_,Ops}=Test|T], Dst) ->
 	false -> [Test|kill_seen_1(T, Dst)]
     end;
 kill_seen_1([], _) -> [].
+
+prune_redundant_values([_Val,F|Vls], F) ->
+    prune_redundant_values(Vls, F);
+prune_redundant_values([Val,Lbl|Vls], F) ->
+    [Val,Lbl|prune_redundant_values(Vls, F)];
+prune_redundant_values([], _) -> [].