compiler: Explain and rename ?BADARG_BLOCK

8 files changed, 38 insertions, 29 deletions

diff --git a/lib/compiler/src/beam_kernel_to_ssa.erl b/lib/compiler/src/beam_kernel_to_ssa.erl
index b2d824b648..474cb1a851 100644
--- a/lib/compiler/src/beam_kernel_to_ssa.erl
+++ b/lib/compiler/src/beam_kernel_to_ssa.erl
@@ -34,7 +34,7 @@
 -type label() :: beam_ssa:label().
 
 %% Main codegen structure.
--record(cg, {lcount=1 :: label(),               %Label counter
+-record(cg, {lcount=1 :: label(),   %Label counter
              bfail=1 :: label(),
              catch_label=none :: 'none' | label(),
              vars=#{} :: map(),     %Defined variables.
@@ -83,6 +83,7 @@ function(#k_fdef{anno=Anno0,func=Name,arity=Arity,
 
 cg_fun(Ke, St0) ->
     {UltimateFail,FailIs,St1} = make_failure(badarg, St0),
+    ?EXCEPTION_BLOCK = UltimateFail,            %Assertion.
     St2 = St1#cg{bfail=UltimateFail,ultimate_failure=UltimateFail},
     {B,St} = cg(Ke, St2),
     Asm = [{label,0}|B++FailIs],
diff --git a/lib/compiler/src/beam_ssa.hrl b/lib/compiler/src/beam_ssa.hrl
index fa76b08453..509a94135e 100644
--- a/lib/compiler/src/beam_ssa.hrl
+++ b/lib/compiler/src/beam_ssa.hrl
@@ -62,5 +62,13 @@
 -record(b_local, {name  :: beam_ssa:b_literal(),
                   arity :: non_neg_integer()}).
 
-%% If this block exists, it calls erlang:error(badarg).
--define(BADARG_BLOCK, 1).
+%% This is a psuedo-block used to express that certain instructions and BIFs
+%% throw exceptions on failure. The code generator rewrites all branches to
+%% this block to {f,0} which causes the instruction to throw an exception
+%% instead of branching.
+%%
+%% Since this is not an ordinary block, it's illegal to merge it with other
+%% blocks, and jumps are only valid when we know that an exception will be
+%% thrown by the operation that branches here; the *block itself* does not
+%% throw an exception.
+-define(EXCEPTION_BLOCK, 1).
diff --git a/lib/compiler/src/beam_ssa_bsm.erl b/lib/compiler/src/beam_ssa_bsm.erl
index 927a06edbf..7a8dc127d7 100644
--- a/lib/compiler/src/beam_ssa_bsm.erl
+++ b/lib/compiler/src/beam_ssa_bsm.erl
@@ -684,9 +684,9 @@ aca_copy_successors(Lbl0, Blocks0, Counter0) ->
     Lbl = maps:get(Lbl0, BRs),
     {Lbl, Blocks, Counter}.
 
-aca_cs_build_brs([?BADARG_BLOCK=Lbl | Path], Counter, Acc) ->
-    %% ?BADARG_BLOCK is a marker and not an actual block, so renaming it will
-    %% break exception handling.
+aca_cs_build_brs([?EXCEPTION_BLOCK=Lbl | Path], Counter, Acc) ->
+    %% ?EXCEPTION_BLOCK is a marker and not an actual block, so renaming it
+    %% will break exception handling.
     aca_cs_build_brs(Path, Counter, Acc#{ Lbl => Lbl });
 aca_cs_build_brs([Lbl | Path], Counter0, Acc) ->
     aca_cs_build_brs(Path, Counter0 + 1, Acc#{ Lbl => Counter0 });
diff --git a/lib/compiler/src/beam_ssa_codegen.erl b/lib/compiler/src/beam_ssa_codegen.erl
index 7102f524d0..10c3419314 100644
--- a/lib/compiler/src/beam_ssa_codegen.erl
+++ b/lib/compiler/src/beam_ssa_codegen.erl
@@ -115,14 +115,14 @@ functions(Forms, AtomMod) ->
 function(#b_function{anno=Anno,bs=Blocks}, AtomMod, St0) ->
     #{func_info:={_,Name,Arity}} = Anno,
     try
-        assert_badarg_block(Blocks),            %Assertion.
+        assert_exception_block(Blocks),            %Assertion.
         Regs = maps:get(registers, Anno),
         St1 = St0#cg{labels=#{},used_labels=gb_sets:empty(),
                      regs=Regs},
         {Fi,St2} = new_label(St1),              %FuncInfo label
         {Entry,St3} = local_func_label(Name, Arity, St2),
         {Ult,St4} = new_label(St3),             %Ultimate failure
-        Labels = (St4#cg.labels)#{0=>Entry,?BADARG_BLOCK=>0},
+        Labels = (St4#cg.labels)#{0=>Entry,?EXCEPTION_BLOCK=>0},
         St5 = St4#cg{labels=Labels,used_labels=gb_sets:singleton(Entry),
                      ultimate_fail=Ult},
         {Body,St} = cg_fun(Blocks, St5#cg{fc_label=Fi}),
@@ -138,10 +138,10 @@ function(#b_function{anno=Anno,bs=Blocks}, AtomMod, St0) ->
             erlang:raise(Class, Error, Stack)
     end.
 
-assert_badarg_block(Blocks) ->
-    %% Assertion: ?BADARG_BLOCK must be the call erlang:error(badarg).
+assert_exception_block(Blocks) ->
+    %% Assertion: ?EXCEPTION_BLOCK must be a call erlang:error(badarg).
     case Blocks of
-        #{?BADARG_BLOCK:=Blk} ->
+        #{?EXCEPTION_BLOCK:=Blk} ->
             #b_blk{is=[#b_set{op=call,dst=Ret,
                               args=[#b_remote{mod=#b_literal{val=erlang},
                                               name=#b_literal{val=error}},
@@ -149,7 +149,7 @@ assert_badarg_block(Blocks) ->
                    last=#b_ret{arg=Ret}} = Blk,
             ok;
         #{} ->
-            %% ?BADARG_BLOCK has been removed because it was never used.
+            %% ?EXCEPTION_BLOCK has been removed because it was never used.
             ok
     end.
 
@@ -631,7 +631,7 @@ liveness_get(S, LiveMap) ->
     end.
 
 liveness_successors(Terminator) ->
-    successors(Terminator) -- [?BADARG_BLOCK].
+    successors(Terminator) -- [?EXCEPTION_BLOCK].
 
 liveness_is([#cg_alloc{}=I0|Is], Regs, Live, Acc) ->
     I = I0#cg_alloc{live=num_live(Live, Regs)},
@@ -965,7 +965,7 @@ cg_block(Is0, Last, Next, St0) ->
     case Last of
         #cg_br{succ=Next,fail=Next} ->
             cg_block(Is0, none, St0);
-        #cg_br{succ=Same,fail=Same} when Same =:= ?BADARG_BLOCK ->
+        #cg_br{succ=Same,fail=Same} when Same =:= ?EXCEPTION_BLOCK ->
             %% An expression in this block *always* throws an exception, so we
             %% terminate it with an 'if_end' to make sure the validator knows
             %% that the following instructions won't actually be reached.
@@ -1839,7 +1839,7 @@ linearize(Blocks) ->
     Linear = beam_ssa:linearize(Blocks),
     linearize_1(Linear, Blocks).
 
-linearize_1([{?BADARG_BLOCK,_}|Ls], Blocks) ->
+linearize_1([{?EXCEPTION_BLOCK,_}|Ls], Blocks) ->
     linearize_1(Ls, Blocks);
 linearize_1([{L,Block0}|Ls], Blocks) ->
     Block = translate_block(L, Block0, Blocks),
diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl
index f77eeecc94..a9f8daaada 100644
--- a/lib/compiler/src/beam_ssa_opt.erl
+++ b/lib/compiler/src/beam_ssa_opt.erl
@@ -2274,7 +2274,7 @@ non_guards_1([{L,#b_blk{is=Is}}|Bs]) ->
             non_guards_1(Bs)
     end;
 non_guards_1([]) ->
-    [?BADARG_BLOCK].
+    [?EXCEPTION_BLOCK].
 
 rel2fam(S0) ->
     S1 = sofs:relation(S0),
diff --git a/lib/compiler/src/beam_ssa_pre_codegen.erl b/lib/compiler/src/beam_ssa_pre_codegen.erl
index ee55d55861..d3cedc3617 100644
--- a/lib/compiler/src/beam_ssa_pre_codegen.erl
+++ b/lib/compiler/src/beam_ssa_pre_codegen.erl
@@ -709,12 +709,12 @@ et_1([L | Ls], Trampolines, Blocks) ->
     case {Is, Last0} of
         {[#b_set{op=exception_trampoline}], #b_br{succ=Succ}} ->
             et_1(Ls, Trampolines#{ L => Succ }, maps:remove(L, Blocks));
-        {_, #b_br{succ=Same,fail=Same}} when Same =:= ?BADARG_BLOCK ->
-            %% The "badarg block" is just a marker saying that we should raise
+        {_, #b_br{succ=Same,fail=Same}} when Same =:= ?EXCEPTION_BLOCK ->
+            %% The exception block is just a marker saying that we should raise
             %% an exception (= {f,0}) instead of jumping to a particular fail
             %% block. Since it's not a reachable block we can't allow
             %% unconditional jumps to it except through a trampoline.
-            error({illegal_jump_to_badarg_block, L});
+            error({illegal_jump_to_exception_block, L});
         {_, #b_br{succ=Succ0,fail=Fail0}} ->
             Succ = maps:get(Succ0, Trampolines, Succ0),
             Fail = maps:get(Fail0, Trampolines, Fail0),
@@ -1338,10 +1338,10 @@ place_frame_here(L, Blocks, Doms, Frames) ->
             Descendants = beam_ssa:rpo([L], Blocks),
             PhiPredecessors = phi_predecessors(L, Blocks),
             MustDominate = ordsets:from_list(PhiPredecessors ++ Descendants),
-            Dominates = all(fun(?BADARG_BLOCK) ->
+            Dominates = all(fun(?EXCEPTION_BLOCK) ->
                                     %% This block defines no variables and calls
                                     %% erlang:error(badarg). It does not matter
-                                    %% whether L dominates ?BADARG_BLOCK or not;
+                                    %% whether L dominates ?EXCEPTION_BLOCK or not;
                                     %% it is still safe to put the frame in L.
                                     true;
                                (Bl) ->
@@ -1823,7 +1823,7 @@ collect_yregs([], Yregs) -> Yregs.
 copy_retval_2([L|Ls], Yregs, Copy0, Blocks0, Count0) ->
     #b_blk{is=Is0,last=Last} = Blk = map_get(L, Blocks0),
     RC = case {Last,Ls} of
-             {#b_br{succ=Succ,fail=?BADARG_BLOCK},[Succ|_]} ->
+             {#b_br{succ=Succ,fail=?EXCEPTION_BLOCK},[Succ|_]} ->
                  true;
              {_,_} ->
                  false
@@ -2562,9 +2562,9 @@ reserve_xregs_is([], Res, Xs, _Used) ->
     {Res,Xs}.
 
 %% Pick up register hints from the successors of this blocks.
-reserve_terminator(_L, _Is, #b_br{bool=#b_var{},succ=Succ,fail=?BADARG_BLOCK},
+reserve_terminator(_L, _Is, #b_br{bool=#b_var{},succ=Succ,fail=?EXCEPTION_BLOCK},
                    _Blocks, XsMap, _Res) ->
-    %% We know that no variables are used at ?BADARG_BLOCK, so
+    %% We know that no variables are used at ?EXCEPTION_BLOCK, so
     %% any register hints from the success blocks are safe to use.
     map_get(Succ, XsMap);
 reserve_terminator(L, Is, #b_br{bool=#b_var{},succ=Succ,fail=Fail},
diff --git a/lib/compiler/src/beam_ssa_share.erl b/lib/compiler/src/beam_ssa_share.erl
index 426efa2cc9..85ab088d14 100644
--- a/lib/compiler/src/beam_ssa_share.erl
+++ b/lib/compiler/src/beam_ssa_share.erl
@@ -117,8 +117,8 @@ share_terminator(_Last, _Blocks) -> none.
 %% possible if the blocks are not equivalent, as that is the common
 %% case.
 
-are_equivalent(_Succ, _, ?BADARG_BLOCK, _, _Blocks) ->
-    %% ?BADARG_BLOCK is special. Sharing could be incorrect.
+are_equivalent(_Succ, _, ?EXCEPTION_BLOCK, _, _Blocks) ->
+    %% ?EXCEPTION_BLOCK is special. Sharing could be incorrect.
     false;
 are_equivalent(_Succ, #b_blk{is=Is1,last=#b_ret{arg=RetVal1}=Ret1},
          _Fail, #b_blk{is=Is2,last=#b_ret{arg=RetVal2}=Ret2}, _Blocks) ->
diff --git a/lib/compiler/src/beam_ssa_type.erl b/lib/compiler/src/beam_ssa_type.erl
index f87acdb500..5e7b54a064 100644
--- a/lib/compiler/src/beam_ssa_type.erl
+++ b/lib/compiler/src/beam_ssa_type.erl
@@ -108,7 +108,7 @@ opt_continue_1(Linear0, Args, Id, Ts, FuncDb0) ->
     D = #d{ func_db=FuncDb0,
             func_id=Id,
             ds=Defs,
-            ls=#{0=>Ts,?BADARG_BLOCK=>#{}},
+            ls=#{0=>Ts,?EXCEPTION_BLOCK=>#{}},
             once=UsedOnce },
 
     {Linear, FuncDb, NewRet} = opt(Linear0, D, []),
@@ -892,8 +892,8 @@ sub_sw_list_1(Type, [{Val,_}|T], Ts) ->
 sub_sw_list_1(Type, [], _Ts) ->
     Type.
 
-update_successor(?BADARG_BLOCK, _Ts, #d{}=D) ->
-    %% We KNOW that no variables are used in the ?BADARG_BLOCK,
+update_successor(?EXCEPTION_BLOCK, _Ts, #d{}=D) ->
+    %% We KNOW that no variables are used in the ?EXCEPTION_BLOCK,
     %% so there is no need to update the type information. That
     %% can be a huge timesaver for huge functions.
     D;