From 39d1a52880ac90f99d5accd8df03a23fbdc05d11 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Thu, 4 Jul 2019 11:30:55 +0200
Subject: beam_ssa_opt: Sink get_tuple_element before type optimization

Eagerly extracting elements can be quite problematic now that we
have union types. Consider the following:

0:
  %% _0 is {ok, #record{}} | {error,atom()}
  _1 = get_tuple_element _0, literal 0
  _2 = get_tuple_element _0, literal 1
  switch _1, label 0, [ { literal ok, label 1 }, { literal error, label 2 } ]
1:
  %% _0 is known to be {ok,#record{}} here
2:
  %% _0 is known to be {error,atom()} here

The type pass is clever enough to see that _0 has the types noted
above, but because the type of _2 is set in the first block where
we didn't have that information yet, it's still #record{} | atom()
in both branches.

Sinking these instructions to when they are used greatly improves
the type information in many cases, but it does have a few
limitations and using it in both of the above branches would take
us back to square one.
---
 lib/compiler/src/beam_ssa_opt.erl | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

(limited to 'lib/compiler/src')

diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl
index a9f8daaada..cce539f582 100644
--- a/lib/compiler/src/beam_ssa_opt.erl
+++ b/lib/compiler/src/beam_ssa_opt.erl
@@ -158,6 +158,7 @@ repeated_passes(Opts) ->
           ?PASS(ssa_opt_dead),
           ?PASS(ssa_opt_cse),
           ?PASS(ssa_opt_tail_phis),
+          ?PASS(ssa_opt_sink),
           ?PASS(ssa_opt_tuple_size),
           ?PASS(ssa_opt_record),
           ?PASS(ssa_opt_type_continue)],        %Must run after ssa_opt_dead to
@@ -175,8 +176,8 @@ epilogue_passes(Opts) ->
           ?PASS(ssa_opt_bsm),
           ?PASS(ssa_opt_bsm_units),
           ?PASS(ssa_opt_bsm_shortcut),
-          ?PASS(ssa_opt_blockify),
           ?PASS(ssa_opt_sink),
+          ?PASS(ssa_opt_blockify),
           ?PASS(ssa_opt_merge_blocks),
           ?PASS(ssa_opt_get_tuple_element),
           ?PASS(ssa_opt_trim_unreachable)],
@@ -1985,9 +1986,7 @@ is_merge_allowed(_, #b_blk{last=#b_switch{}}, #b_blk{}) ->
 %%% extracted values.
 %%%
 
-ssa_opt_sink({#st{ssa=Blocks0}=St, FuncDb}) ->
-    Linear = beam_ssa:linearize(Blocks0),
-
+ssa_opt_sink({#st{ssa=Linear}=St, FuncDb}) ->
     %% Create a map with all variables that define get_tuple_element
     %% instructions. The variable name map to the block it is defined in.
     case def_blocks(Linear) of
@@ -1996,10 +1995,12 @@ ssa_opt_sink({#st{ssa=Blocks0}=St, FuncDb}) ->
             {St, FuncDb};
         [_|_]=Defs0 ->
             Defs = maps:from_list(Defs0),
-            {do_ssa_opt_sink(Linear, Defs, St), FuncDb}
+            {do_ssa_opt_sink(Defs, St), FuncDb}
     end.
 
-do_ssa_opt_sink(Linear, Defs, #st{ssa=Blocks0}=St) ->
+do_ssa_opt_sink(Defs, #st{ssa=Linear}=St) ->
+    Blocks0 = maps:from_list(Linear),
+
     %% Now find all the blocks that use variables defined by get_tuple_element
     %% instructions.
     Used = used_blocks(Linear, Defs, []),
@@ -2024,7 +2025,8 @@ do_ssa_opt_sink(Linear, Defs, #st{ssa=Blocks0}=St) ->
                            From = map_get(V, Defs),
                            move_defs(V, From, To, A)
                    end, Blocks0, DefLoc),
-    St#st{ssa=Blocks}.
+
+    St#st{ssa=beam_ssa:linearize(Blocks)}.
 
 def_blocks([{L,#b_blk{is=Is}}|Bs]) ->
     def_blocks_is(Is, L, def_blocks(Bs));
-- 
cgit v1.2.3