Merge branch 'bjorn/compiler/beam_ssa_bool/ERL-1253/OTP-16657' into maint-23

* bjorn/compiler/beam_ssa_bool/ERL-1253/OTP-16657:
  Avoid unsafe optimization of guards

# Conflicts:
#	lib/compiler/test/guard_SUITE.erl

2 files changed, 211 insertions, 0 deletions

diff --git a/lib/compiler/src/beam_ssa_bool.erl b/lib/compiler/src/beam_ssa_bool.erl
index 8fb87391c5..5b81ca2be1 100644
--- a/lib/compiler/src/beam_ssa_bool.erl
+++ b/lib/compiler/src/beam_ssa_bool.erl
@@ -1072,7 +1072,60 @@ redirect_phi(Phi, Args, SuccFail, G0, St) ->
 redirect_phi_1(PhiVtx, [{#b_literal{val=false},FalseExit},
                         {#b_var{}=SuccBool,_BoolExit}],
              SuccFail, G0, St) ->
+    %% This was most likely an `andalso` in the source code.
     BoolVtx = get_vertex(SuccBool, St),
+
+    %% We must be careful when rewriting guards that reference boolean
+    %% expressions defined before the guard. Here is an example:
+    %%
+    %%    Bool = Z =:= false,
+    %%    if
+    %%      X =:= Y andalso Bool -> ok;
+    %%      true -> error
+    %%    end.
+    %%
+    %% Slightly simplified, the SSA code will look like this:
+    %%
+    %%  10:  Bool = bif:'=:=' _2, `false`
+    %%       br ^11
+    %%
+    %%  11:  B = bif:'=:=' X, Y
+    %%       br B, ^20, ^30
+    %%
+    %%  20:  br ^40
+    %%  30:  br ^40
+    %%
+    %%  40:  Phi = phi { `true`, ^20 }, { Bool, ^30 }
+    %%       br Phi, ^100, ^200
+    %%
+    %%  100: ret `ok`
+    %%  200: ret `error'
+    %%
+    %% The usual rewriting of the phi node will result in the following
+    %% SSA code:
+    %%
+    %%  10:  Bool = bif:'=:=' _2, `false`
+    %%       br Bool, ^100, ^200
+    %%
+    %%  11:  B = bif:'=:=' X, Y
+    %%       br B, ^100, ^200
+    %%
+    %%  20:  br ^40
+    %%  30:  br ^40
+    %%
+    %%  40:  Phi = phi { `true`, ^20 }, { Bool, ^30 }
+    %%       br Phi, ^100, ^200
+    %%
+    %%  100: ret `ok`
+    %%  200: ret `error'
+    %%
+    %% Block 11 is no longer reachable; thus, the X =:= Y test has been dropped.
+    %% To avoid dropping tests, we should check whether if there is a path from
+    %% 10 to block 20. If there is, the optimization in its current form is not
+    %% safe.
+    %%
+    ensure_disjoint_paths(G0, BoolVtx, FalseExit),
+
     [FalseOut] = beam_digraph:out_edges(G0, FalseExit),
     G1 = beam_digraph:del_edge(G0, FalseOut),
     case SuccFail of
@@ -1090,6 +1143,11 @@ redirect_phi_1(PhiVtx, [{#b_literal{val=true},TrueExit},
              {fail,Fail}, G0, St) ->
     %% This was probably an `orelse` in the source code.
     BoolVtx = get_vertex(SuccBool, St),
+
+    %% See the previous clause for an explanation of why we
+    %% must ensure that paths are disjoint.
+    ensure_disjoint_paths(G0, BoolVtx, TrueExit),
+
     [TrueOut] = beam_digraph:out_edges(G0, TrueExit),
     G1 = beam_digraph:del_edge(G0, TrueOut),
     G2 = beam_digraph:add_edge(G1, TrueExit, PhiVtx, next),
@@ -1119,6 +1177,18 @@ digraph_bool_def(G) ->
     Ds = [{Dst,Vtx} || {Vtx,#b_set{dst=Dst}} <- Vs],
     maps:from_list(Ds).
 
+
+%% ensure_disjoint_paths(G, Vertex1, Vertex2) -> ok.
+%%  Ensure that there is no path from Vertex1 to Vertex2 in
+%%  either direction. (It is probably overkill to test both
+%%  directions, but better safe than sorry.)
+
+ensure_disjoint_paths(G, V1, V2) ->
+    case beam_digraph:is_path(G, V1, V2) orelse beam_digraph:is_path(G, V2, V1) of
+        true -> not_possible();
+        false -> ok
+    end.
+
 %%%
 %%% Shortcut branches that branch to other branches.
 %%%
diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl
index badbde612e..2c935c1c57 100644
--- a/lib/compiler/test/guard_SUITE.erl
+++ b/lib/compiler/test/guard_SUITE.erl
@@ -2300,6 +2300,7 @@ beam_bool_SUITE(_Config) ->
     in_catch(),
     recv_semi(),
     andalso_repeated_var(),
+    erl1253(),
     erl1246(),
     ok.
 
@@ -2622,6 +2623,146 @@ erl1246_conf(gift_coll) -> {9502, {112, 45}};
 erl1246_conf(transform_id) -> 12;
 erl1246_conf(_) -> undefined.
 
+erl1253() ->
+    ok = erl1253_orelse_false(a, a, any),
+    ok = erl1253_orelse_false(a, a, true),
+    ok = erl1253_orelse_false(a, a, false),
+    error = erl1253_orelse_false(a, b, any),
+    error = erl1253_orelse_false(a, b, true),
+    ok = erl1253_orelse_false(a, b, false),
+
+    ok = erl1253_orelse_true(a, a, any),
+    ok = erl1253_orelse_true(a, a, true),
+    ok = erl1253_orelse_true(a, a, false),
+    error = erl1253_orelse_true(a, b, any),
+    ok = erl1253_orelse_true(a, b, true),
+    error = erl1253_orelse_true(a, b, false),
+
+    error = erl1253_andalso_false(a, a, any),
+    error = erl1253_andalso_false(a, a, true),
+    ok = erl1253_andalso_false(a, a, false),
+    error = erl1253_andalso_false(a, b, any),
+    error = erl1253_andalso_false(a, b, true),
+    error = erl1253_andalso_false(a, b, false),
+
+    error = erl1253_andalso_true(a, a, any),
+    ok = erl1253_andalso_true(a, a, true),
+    error = erl1253_andalso_true(a, a, false),
+    error = erl1253_andalso_true(a, b, any),
+    error = erl1253_andalso_true(a, b, true),
+    error = erl1253_andalso_true(a, b, false),
+
+    ok.
+
+erl1253_orelse_false(X, Y, Z) ->
+    Res = erl1253_orelse_false_1(X, Y, Z),
+    Res = erl1253_orelse_false_2(X, Y, Z),
+    Res = erl1253_orelse_false_3(X, Y, Z).
+
+erl1253_orelse_false_1(X, Y, Z) ->
+    Bool = Z =:= false,
+    if
+        X =:= Y orelse Bool -> ok;
+        true -> error
+    end.
+
+erl1253_orelse_false_2(X, Y, Z) ->
+    Bool = Z =:= false,
+    if
+        Bool orelse X =:= Y -> ok;
+        true -> error
+    end.
+
+erl1253_orelse_false_3(X, Y, Z) ->
+    Bool1 = X =:= Y,
+    Bool2 = Z =:= false,
+    if
+        Bool1 orelse Bool2 -> ok;
+        true -> error
+    end.
+
+erl1253_orelse_true(X, Y, Z) ->
+    Res = erl1253_orelse_true_1(X, Y, Z),
+    Res = erl1253_orelse_true_2(X, Y, Z),
+    Res = erl1253_orelse_true_3(X, Y, Z).
+
+erl1253_orelse_true_1(X, Y, Z)   ->
+    Bool = Z =:= true,
+    if
+        X =:= Y orelse Bool -> ok;
+        true -> error
+    end.
+
+erl1253_orelse_true_2(X, Y, Z) ->
+    Bool = Z =:= true,
+    if
+        Bool orelse X =:= Y -> ok;
+        true -> error
+    end.
+
+erl1253_orelse_true_3(X, Y, Z)   ->
+    Bool1 = X =:= Y,
+    Bool2 = Z =:= true,
+    if
+        Bool1 orelse Bool2 -> ok;
+        true -> error
+    end.
+
+erl1253_andalso_false(X, Y, Z) ->
+    Res = erl1253_andalso_false_1(X, Y, Z),
+    Res = erl1253_andalso_false_2(X, Y, Z),
+    Res = erl1253_andalso_false_3(X, Y, Z).
+
+erl1253_andalso_false_1(X, Y, Z) ->
+    Bool = Z =:= false,
+    if
+        X =:= Y andalso Bool -> ok;
+        true -> error
+    end.
+
+erl1253_andalso_false_2(X, Y, Z) ->
+    Bool1 = X =:= Y,
+    Bool2 = Z =:= false,
+    if
+        Bool1 andalso Bool2 -> ok;
+        true -> error
+    end.
+
+erl1253_andalso_false_3(X, Y, Z) ->
+    Bool1 = X =:= Y,
+    Bool2 = Z =:= false,
+    if
+        Bool1 andalso Bool2 -> ok;
+        true -> error
+    end.
+
+erl1253_andalso_true(X, Y, Z) ->
+    Res = erl1253_andalso_true_1(X, Y, Z),
+    Res = erl1253_andalso_true_2(X, Y, Z),
+    Res = erl1253_andalso_true_3(X, Y, Z).
+
+erl1253_andalso_true_1(X, Y, Z) ->
+    Bool = Z =:= true,
+    if
+        X =:= Y andalso Bool -> ok;
+        true -> error
+    end.
+
+erl1253_andalso_true_2(X, Y, Z) ->
+    Bool = Z =:= true,
+    if
+        Bool andalso X =:= Y-> ok;
+        true -> error
+    end.
+
+erl1253_andalso_true_3(X, Y, Z) ->
+    Bool1 = X =:= Y,
+    Bool2 = Z =:= true,
+    if
+        Bool1 andalso Bool2 -> ok;
+        true -> error
+    end.
+
 %%%
 %%% End of beam_bool_SUITE tests.
 %%%