File 2073-beam_ssa-Optimize-merge_blocks-1.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2073-beam_ssa-Optimize-merge_blocks-1.patch of Package erlang

From 670c5319d291afec77d959cda81102325520afa9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 28 May 2020 14:26:26 +0200
Subject: [PATCH 3/3] beam_ssa: Optimize merge_blocks/1

Updating the predecessors for blocks with multiple predecessors
would waste a lot of time for some modules such as elixir's
String.Unicode module.
---
 lib/compiler/src/beam_ssa.erl     | 51 ++++++++++++++++++++++---------
 lib/compiler/test/guard_SUITE.erl |  4 +++
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/lib/compiler/src/beam_ssa.erl b/lib/compiler/src/beam_ssa.erl
index a978d107b1..64aa8a3359 100644
--- a/lib/compiler/src/beam_ssa.erl
+++ b/lib/compiler/src/beam_ssa.erl
@@ -643,6 +643,10 @@ fold_uses_block(Lbl, #b_blk{is=Is,last=Last}, UseMap0) ->
 
 merge_blocks(Blocks) ->
     Preds = predecessors(Blocks),
+
+    %% We must traverse the blocks in reverse postorder to avoid
+    %% embedding succeeded:guard instructions into the middle of
+    %% blocks when this function is called from beam_ssa_bool.
     merge_blocks_1(rpo(Blocks), Preds, Blocks).
 
 %%%
@@ -941,9 +945,8 @@ merge_blocks_1([L|Ls], Preds0, Blocks0) ->
                     #b_blk{is=Is0} = Blk0,
                     #b_blk{is=Is1} = Blk1,
                     verify_merge_is(Is1),
-                    Is = Is0 ++ Is1,
-                    Blk2 = Blk1#b_blk{is=Is},
-                    Blk = merge_fix_succeeded(Blk2),
+                    Is = merge_fix_succeeded(Is0 ++ Is1, Blk1),
+                    Blk = Blk1#b_blk{is=Is},
                     Blocks1 = maps:remove(L, Blocks0),
                     Blocks2 = Blocks1#{P:=Blk},
                     Successors = successors(Blk),
@@ -958,24 +961,44 @@ merge_blocks_1([L|Ls], Preds0, Blocks0) ->
     end;
 merge_blocks_1([], _Preds, Blocks) -> Blocks.
 
+%% Since we process the candidates in reverse postorder it is necessary
+%% to update the predecessors. Reverse postorder is necessary to ensure
+%% that merge_fix_succeeded/2 will find and remove all succeeded:guard
+%% not followed by a two-way branch.
 merge_update_preds([L|Ls], From, To, Preds0) ->
-    Ps = [rename_label(P, From, To) || P <- map_get(L, Preds0)],
-    Preds = Preds0#{L:=Ps},
-    merge_update_preds(Ls, From, To, Preds);
+    case Preds0 of
+        #{L := [P]} ->
+            Preds = Preds0#{L := [rename_label(P, From, To)]},
+            merge_update_preds(Ls, From, To, Preds);
+        #{} ->
+            %% More than one predecessor, so this block will not be
+            %% merged. Updating the predecessors is not needed and
+            %% updating would waste a lot of time if there are many
+            %% predecessors.
+            merge_update_preds(Ls, From, To, Preds0)
+    end;
 merge_update_preds([], _, _, Preds) -> Preds.
 
-merge_fix_succeeded(#b_blk{is=[_|_]=Is0,last=#b_br{succ=To,fail=To}}=Blk) ->
+merge_fix_succeeded(Is, #b_blk{last=#b_br{succ=Succ,fail=Fail}}) when Succ =/= Fail ->
+    %% This is a two-way branch. There is no need look at the instructions.
+    Is;
+merge_fix_succeeded([_|_]=Is0, #b_blk{}) ->
+    %% Not a two-way branch.
     case reverse(Is0) of
         [#b_set{op={succeeded,guard},args=[Dst]},#b_set{dst=Dst}|Is] ->
-            %% A succeeded:guard instruction must not be followed by a one-way
-            %% branch. Eliminate it. (We do this mainly for the benefit of the
-            %% beam_ssa_bool pass. When called from beam_ssa_opt there should
-            %% be no such instructions left.)
-            Blk#b_blk{is=reverse(Is)};
+            %% This succeeded:guard instruction MUST be followed by a
+            %% two-way branch. It is not, which means that its result
+            %% will never be used. Therefore, the instruction and
+            %% succeeded:guard must be removed.
+            %%
+            %% We remove those instructions for the benefit of the
+            %% beam_ssa_bool pass. When called from beam_ssa_opt there
+            %% should be no such instructions left.
+            reverse(Is);
         _ ->
-            Blk
+            Is0
     end;
-merge_fix_succeeded(Blk) -> Blk.
+merge_fix_succeeded(Is, _Blk) -> Is.
 
 verify_merge_is([#b_set{op=Op}|_]) ->
     %% The merged block has only one predecessor, so it should not have any phi
diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl
index 05ec491efc..dba571f80f 100644
--- a/lib/compiler/test/guard_SUITE.erl
+++ b/lib/compiler/test/guard_SUITE.erl
@@ -2601,6 +2601,10 @@ fail_in_guard() ->
     error = fun() when (0 #fail_in_guard.f)#fail_in_guard.f -> ok;
                () -> error
             end(),
+    error = fun() when 42; <<0.5,0:(element(true, false))>> ->
+                    a = b;
+               () -> error
+            end(),
 
     ok.
 
-- 
2.26.2

Places

File 2073-beam_ssa-Optimize-merge_blocks-1.patch of Package erlang

Places