From 4d6e4068ecc88661a1395364091e96683b29fc7d Mon Sep 17 00:00:00 2001
From: Frej Drejhammar <frej.drejhammar@gmail.com>
Date: Mon, 25 Nov 2024 15:31:19 +0100
Subject: [PATCH] compiler destructive update: Handle nested patches for phis

The code responsible for patching Phi-instructions could not handle
multiple patches to a single literal value, it silently discarded all
but one of the updates. This MR corrects this by merging patches,
using the same mechanism as used for patching function arguments.

Closes #9100
---
 .../src/beam_ssa_destructive_update.erl       | 14 ++++--
 .../private_append.erl                        | 43 ++++++++++++++++++-
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/lib/compiler/src/beam_ssa_destructive_update.erl b/lib/compiler/src/beam_ssa_destructive_update.erl
index d0b6a773bef4..b5fb8dbae076 100644
--- a/lib/compiler/src/beam_ssa_destructive_update.erl
+++ b/lib/compiler/src/beam_ssa_destructive_update.erl
@@ -862,9 +862,8 @@ merge_patches({self,heap_tuple}, Other) ->
     Other.
 
 patch_phi(I0=#b_set{op=phi,args=Args0}, Patches, Cnt0) ->
-    L2P = foldl(fun(Phi={phi,_,Lbl,_,_}, Acc) ->
-                        Acc#{Lbl => Phi}
-                end, #{}, Patches),
+    ?DP("Patching Phi:~n args: ~p~n  patches: ~p~n", [Args0, Patches]),
+    L2P = foldl(fun merge_phi_patch/2, #{}, Patches),
     {Args, Extra, Cnt} =
         foldr(fun(Arg0={_,Lbl}, {ArgsAcc,ExtraAcc,CntAcc}) ->
                       case L2P of
@@ -880,6 +879,15 @@ patch_phi(I0=#b_set{op=phi,args=Args0}, Patches, Cnt0) ->
     I = I0#b_set{op=phi,args=Args},
     {I, Extra, Cnt}.
 
+merge_phi_patch({phi,Var,Lbl,Lit,E}, Acc) ->
+    case Acc of
+        #{Lbl:={phi,Var,Lbl,Lit,Old}} ->
+            Acc#{Lbl => {phi,Var,Lbl,Lit,merge_patches(E, Old)}};
+        #{} ->
+            false = is_map_key(Lbl, Acc), %% Assert
+            Acc#{Lbl => {phi,Var,Lbl,Lit,E}}
+    end.
+
 %% Should return the instructions in reversed order
 patch_literal_term(Tuple, {tuple_elements,Elems}, Cnt) ->
     Es = [{tuple_element,I,E,0} || {I,E} <- keysort(1, Elems)],
diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/private_append.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/private_append.erl
index 40baae83b856..589084e902dd 100644
--- a/lib/compiler/test/beam_ssa_check_SUITE_data/private_append.erl
+++ b/lib/compiler/test/beam_ssa_check_SUITE_data/private_append.erl
@@ -81,7 +81,9 @@
          bs_create_bin_on_literal/0,
 
          crash_in_value_tracking/3,
-         crash_in_value_tracking_inner/3]).
+         crash_in_value_tracking_inner/3,
+
+         gh9100/0]).
 
 %% Trivial smoke test
 transformable0(L) ->
@@ -1035,3 +1037,42 @@ crash_in_value_tracking(_, _V0, _) ->
     ((<<((crash_in_value_tracking_inner(
             {#{#{ ok => ok || _ := _ <- ok} => ok},
              _V0, false, _V0, "Bo"}, _V0, ok)))/bytes>>) =/= ok).
+
+gh9100() ->
+    gh9100(#{prev => nil,
+	     next =>
+		 [{equal, <<"a">>},
+		  {delete, <<"y">>}]},
+	   {{<<>>, <<>>}}).
+
+%% We could fail to update multiple elements of a tuple, it was a
+%% literal tuple in a Phi-instruction.
+gh9100(#{next := [{Op, Text} | Next]} = Diffs,
+       {{TextDelete, TextInsert}}) ->
+%ssa% (_, Acc) when post_ssa_opt ->
+%ssa% switch(X, Fail, [{'delete',_},{'equal',Equal},...]),
+%ssa% label Equal,
+%ssa% A = bs_init_writable(_),
+%ssa% B = bs_init_writable(_),
+%ssa% C = put_tuple(A, B),
+%ssa% D = put_tuple(C).
+    Acc =
+        case Op of
+            insert ->
+                {{TextDelete,
+		  <<TextInsert/binary,Text/binary>>}};
+            delete ->
+                {{<<TextDelete/binary,Text/binary>>,
+		  TextInsert}};
+            equal ->
+                {{<<>>, <<>>}}   %% Bug is here.
+        end,
+    gh9100(#{prev =>
+		 case Diffs of
+		     #{prev := Prev} ->
+			 Prev;
+		     Other ->
+                            ex:no_parens_remote(Other)
+		 end,
+	     next => Next},
+	   Acc).