File 0196-beam_ssa_share-Mend-sharing-optimization-f... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 0196-beam_ssa_share-Mend-sharing-optimization-for-tail-ca.patch of Package erlang

From 555e2a72956c9871f42135681a45bcc1960b5995 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 24 Mar 2022 12:53:53 +0100
Subject: [PATCH] beam_ssa_share: Mend sharing optimization for tail calls

72878b61e41e7a introduced `{succeeded,body}` instructions after
tail calls. With that change, the `beam_ssa_share` pass would never
share blocks ending with tail calls. This commit mends the optimization.
---
 lib/compiler/src/beam_ssa_share.erl  | 36 ++++++++++++++++++----------
 lib/compiler/test/beam_ssa_SUITE.erl | 12 +++++++++-
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/lib/compiler/src/beam_ssa_share.erl b/lib/compiler/src/beam_ssa_share.erl
index bf211ff32b..bd21655914 100644
--- a/lib/compiler/src/beam_ssa_share.erl
+++ b/lib/compiler/src/beam_ssa_share.erl
@@ -297,7 +297,7 @@ canonical_is(Is) ->
     Can.
 
 canonical_is([#b_set{op=Op,dst=Dst,args=Args0}=I|Is], VarMap0, Acc) ->
-    Args = [canonical_arg(Arg, VarMap0) || Arg <-Args0],
+    Args = [canonical_arg(Arg, VarMap0) || Arg <- Args0],
     Var = {var,map_size(VarMap0)},
     VarMap = VarMap0#{Dst=>Var},
     LineAnno = case Op of
@@ -312,17 +312,8 @@ canonical_is([#b_set{op=Op,dst=Dst,args=Args0}=I|Is], VarMap0, Acc) ->
                        beam_ssa:get_anno(location, I, none)
                end,
     canonical_is(Is, VarMap, {Op,LineAnno,Var,Args,Acc});
-canonical_is([#b_ret{arg=Arg}], VarMap, Acc0) ->
-    Acc1 = case Acc0 of
-               {call,_Anno,Var,[#b_local{}|_]=Args,PrevAcc} ->
-                   %% This is a tail-recursive call to a local function.
-                   %% There will be no line instruction generated;
-                   %% thus, the annotation is not significant.
-                   {call,[],Var,Args,PrevAcc};
-               _ ->
-                   Acc0
-           end,
-    {{ret,canonical_arg(Arg, VarMap),Acc1},VarMap};
+canonical_is([#b_ret{arg=Arg}], VarMap, Acc) ->
+    {{ret,canonical_arg(Arg, VarMap),Acc},VarMap};
 canonical_is([#b_br{bool=#b_var{}=Arg,fail=Fail}], VarMap, Acc) ->
     %% A previous buggy version of this code omitted the canonicalized
     %% argument in the return value. Unfortunately, that worked most
@@ -331,6 +322,19 @@ canonical_is([#b_br{bool=#b_var{}=Arg,fail=Fail}], VarMap, Acc) ->
     {{br,canonical_arg(Arg, VarMap),succ,Fail,Acc},VarMap};
 canonical_is([#b_br{succ=Succ}], VarMap, Acc) ->
     {{br,Succ,Acc},VarMap};
+canonical_is([{tail_br,Arg0,Ret0}], VarMap, Acc0) ->
+    Arg = canonical_arg(Arg0, VarMap),
+    Ret = canonical_arg(Ret0, VarMap),
+    case Acc0 of
+        {{succeeded,body},_,Arg,[Ret],{call,_,Ret,CallArgs,PrevAcc}} ->
+            %% This is a tail-recursive call to a local function.
+            %% There will be no line instruction generated; thus, the
+            %% annotation is not significant.
+            {{tail_call,Ret,CallArgs,PrevAcc},VarMap};
+        _ ->
+            %% Not a tail-recursive call.
+            {{br_ret,Arg,Ret,Acc0},VarMap}
+    end;
 canonical_is([], VarMap, Acc) ->
     {Acc,VarMap}.
 
@@ -344,6 +348,14 @@ canonical_terminator(L, #b_br{bool=#b_literal{val=true},succ=Succ}=Br, Blocks) -
         [_|_]=Phis ->
             {Phis ++ [Br],done}
     end;
+canonical_terminator(_L, #b_br{bool=#b_var{}=Arg,succ=Succ,fail=?EXCEPTION_BLOCK}=Br,
+                     Blocks) ->
+    case Blocks of
+        #{Succ := #b_blk{is=[],last=#b_ret{arg=Ret}}} ->
+            {[{tail_br,Arg,Ret}],done};
+        #{} ->
+            {[Br],Succ}
+    end;
 canonical_terminator(_L, #b_br{bool=#b_var{},succ=Succ}=Br, _Blocks) ->
     {[Br],Succ};
 canonical_terminator(_, _, _) -> none.
diff --git a/lib/compiler/test/beam_ssa_SUITE.erl b/lib/compiler/test/beam_ssa_SUITE.erl
index 9e2f7ab1a1..2f29ee27f9 100644
--- a/lib/compiler/test/beam_ssa_SUITE.erl
+++ b/lib/compiler/test/beam_ssa_SUITE.erl
@@ -669,6 +669,7 @@ do_comb_sw_2(X) ->
 share_opt(_Config) ->
     ok = do_share_opt_1(0),
     ok = do_share_opt_2(),
+    ok = do_share_opt_3(),
     ok.
 
 do_share_opt_1(A) ->
@@ -699,7 +700,16 @@ sopt_2({Flags, Opts}, ok) ->
         [] ->
             ok
     end.
-    
+
+do_share_opt_3() ->
+    true = sopt_3(id(ok)),
+    false = sopt_3(id(nok)),
+    ok.
+
+sopt_3(X) ->
+    %% Must be one line to trigger bug.
+    case X of ok -> id(?LINE), true; _ -> id(?LINE), false end.
+
 beam_ssa_dead_crash(_Config) ->
     not_A_B = do_beam_ssa_dead_crash(id(false), id(true)),
     not_A_not_B = do_beam_ssa_dead_crash(false, false),
-- 
2.34.1

Places

File 0196-beam_ssa_share-Mend-sharing-optimization-for-tail-ca.patch of Package erlang

Places