Cfg_simplify pass (#3768)

* Move all dead code passes into Cfg_simplify and delete trap handlers
* Fix catch-try*.cmm tests
* Add compilation flag -cfg-eliminate-dead-trap-handlers
* Add flag cfg-eliminate-dead-trap-handlers to some github ci jobs

Author: gretay-js

.github/workflows/build.yml

@@ -65,7 +65,7 @@ jobs:
             os: ubuntu-latest
             build_ocamlparam: ''
             use_runtime: d
-            ocamlparam: '_,O3=1,flambda2-expert-cont-lifting-budget=200'
+            ocamlparam: '_,O3=1,flambda2-expert-cont-lifting-budget=200,cfg-invariants=1,cfg-eliminate-dead-trap-handlers=1'
 
           - name: flambda2_frame_pointers_oclassic_polling
             config: --enable-middle-end=flambda2 --enable-frame-pointers --enable-poll-insertion --enable-flambda-invariants
@@ -164,8 +164,8 @@ jobs:
           - name: cfg-invariants
             config: --enable-middle-end=flambda2
             os: ubuntu-latest
-            build_ocamlparam: '_,w=-46,regalloc=cfg'
-            ocamlparam: '_,w=-46,regalloc=cfg,cfg-invariants=1'
+            build_ocamlparam: '_,w=-46,regalloc=cfg,cfg-invariants=1,cfg-eliminate-dead-trap-handlers=1'
+            ocamlparam: '_,w=-46,regalloc=cfg,cfg-invariants=1,cfg-eliminate-dead-trap-handlers=1'
             check_arch: true
 
           - name: vectorizer

backend/amd64/proc.ml

@@ -596,7 +596,7 @@ let destroyed_at_basic (basic : Cfg_intf.S.basic) =
                   | Isextend32 | Izextend32 | Ipause
                   | Ilfence | Isfence | Imfence)
        | Name_for_debugger _ | Dls_get)
-  | Poptrap | Prologue ->
+  | Poptrap _ | Prologue ->
     if fp then [| rbp |] else [||]
   | Stack_check _ ->
     assert false (* the instruction is added after register allocation *)

backend/amd64/regalloc_stack_operands.ml

@@ -255,7 +255,7 @@ let basic (map : spilled_map) (instr : Cfg.basic Cfg.instruction) =
                  | Ibswap _))
   | Reloadretaddr
   | Pushtrap _
-  | Poptrap
+  | Poptrap _
   | Prologue ->
     (* no rewrite *)
     May_still_have_spilled_registers

backend/arm64/proc.ml

@@ -388,7 +388,7 @@ let destroyed_at_basic (basic : Cfg_intf.S.basic) =
         | Intop _ | Intop_imm _ | Intop_atomic _
         | Name_for_debugger _ | Probe_is_enabled _ | Opaque
         | Begin_region | End_region | Dls_get)
-  | Poptrap | Prologue
+  | Poptrap _ | Prologue
     -> [||]
   | Stack_check _ -> assert false (* not supported *)
 

backend/cfg/cfg.ml

@@ -43,7 +43,6 @@ type basic_block =
     mutable exn : Label.t option;
     mutable can_raise : bool;
     mutable is_trap_handler : bool;
-    mutable dead : bool;
     mutable cold : bool
   }
 
@@ -187,17 +186,43 @@ let add_block_exn t block =
       Label.format block.start;
   Label.Tbl.add t.blocks block.start block
 
-let remove_block_exn t label =
-  match Label.Tbl.find t.blocks label with
-  | exception Not_found ->
-    Misc.fatal_errorf "Cfg.remove_block_exn: block %a not found" Label.format
-      label
-  | _ -> Label.Tbl.remove t.blocks label
+let remove_trap_instructions t removed_trap_handlers =
+  let remove_trap_instr _ b =
+    DLL.iter_cell b.body ~f:(fun cell ->
+        let i = DLL.value cell in
+        match i.desc with
+        | Pushtrap { lbl_handler } | Poptrap { lbl_handler } ->
+          if Label.Set.mem lbl_handler removed_trap_handlers
+          then DLL.delete_curr cell
+        | Op _ | Reloadretaddr | Prologue | Stack_check _ -> ())
+  in
+  if not (Label.Set.is_empty removed_trap_handlers)
+  then (
+    (* CR-someday gyorsh: avoid iterating over all the instructions to just to
+       remove a few pushtrap/poptrap. *)
+    assert Config.flambda2;
+    (* remove Lpushtrap and Lpoptrap instructions that refer to dead labels. *)
+    Label.Tbl.iter remove_trap_instr t.blocks)
 
 let remove_blocks t labels_to_remove =
+  let removed_labels = ref Label.Set.empty in
+  let removed_trap_handlers = ref Label.Set.empty in
   Label.Tbl.filter_map_inplace
-    (fun l b -> if Label.Set.mem l labels_to_remove then None else Some b)
-    t.blocks
+    (fun l b ->
+      if Label.Set.mem l labels_to_remove
+      then (
+        if b.is_trap_handler
+        then removed_trap_handlers := Label.Set.add l !removed_trap_handlers;
+        removed_labels := Label.Set.add l !removed_labels;
+        None)
+      else Some b)
+    t.blocks;
+  let labels_not_found = Label.Set.diff labels_to_remove !removed_labels in
+  if not (Label.Set.is_empty labels_not_found)
+  then
+    Misc.fatal_errorf "Cfg.remove_blocks: not found blocks %a" Label.Set.print
+      labels_not_found;
+  remove_trap_instructions t !removed_trap_handlers
 
 let get_block t label = Label.Tbl.find_opt t.blocks label
 
@@ -262,7 +287,8 @@ let dump_basic ppf (basic : basic) =
   | Reloadretaddr -> fprintf ppf "Reloadretaddr"
   | Pushtrap { lbl_handler } ->
     fprintf ppf "Pushtrap handler=%a" Label.format lbl_handler
-  | Poptrap -> fprintf ppf "Poptrap"
+  | Poptrap { lbl_handler } ->
+    fprintf ppf "Poptrap handler=%a" Label.format lbl_handler
   | Prologue -> fprintf ppf "Prologue"
   | Stack_check { max_frame_size_bytes } ->
     fprintf ppf "Stack_check size=%d" max_frame_size_bytes
@@ -449,7 +475,7 @@ let is_pure_basic : basic -> bool = function
        wouldn't be. Saying it's not pure doesn't decrease the generated code
        quality and is future-proof.*)
     false
-  | Pushtrap _ | Poptrap ->
+  | Pushtrap _ | Poptrap _ ->
     (* Those instructions modify the trap stack which actually modifies the
        stack pointer. *)
     false
@@ -491,7 +517,7 @@ let is_noop_move instr =
       | Intop_imm _ | Intop_atomic _ | Floatop _ | Opaque | Reinterpret_cast _
       | Static_cast _ | Probe_is_enabled _ | Specific _ | Name_for_debugger _
       | Begin_region | End_region | Dls_get | Poll | Alloc _ )
-  | Reloadretaddr | Pushtrap _ | Poptrap | Prologue | Stack_check _ ->
+  | Reloadretaddr | Pushtrap _ | Poptrap _ | Prologue | Stack_check _ ->
     false
 
 let set_stack_offset (instr : _ instruction) stack_offset =
@@ -564,7 +590,6 @@ let make_empty_block ?label terminator : basic_block =
     exn = None;
     can_raise = false;
     is_trap_handler = false;
-    dead = false;
     cold = false
   }
 

backend/cfg/cfg.mli

@@ -54,16 +54,12 @@ type basic_block =
     mutable is_trap_handler : bool;
         (** Is this block a trap handler (i.e. is it an exn successor of another
             block) or not? *)
-    mutable dead : bool;
-        (** This block must be unreachable from function entry. This field is
-            set during cfg construction (if trap stacks are unresolved) and used
-            during dead block elimination for checking. *)
     mutable cold : bool
         (* CR-someday gyorsh: The current implementation allows multiple
            pushtraps in each block means that different trap stacks are
            associated with the block at different points. At most one
-           instruction in each block can raise, and always the last one. After
-           we split the blocks based on Pushtrap/Poptrap, each block will have a
+           instruction in each block can raise, and always the last one. If we
+           split the blocks based on Pushtrap/Poptrap, each block will have a
            unique trap stack associated with it. [exns] will not be needed, as
            the exn-successor will be uniquely determined by can_raise + top of
            trap stack. *)
@@ -143,8 +139,6 @@ val mem_block : t -> Label.t -> bool
 
 val add_block_exn : t -> basic_block -> unit
 
-val remove_block_exn : t -> Label.t -> unit
-
 val remove_blocks : t -> Label.Set.t -> unit
 
 val get_block : t -> Label.t -> basic_block option

backend/cfg/cfg_available_regs.ml

@@ -309,7 +309,7 @@ module Transfer = struct
             | Reinterpret_cast _ | Static_cast _ | Probe_is_enabled _ | Opaque
             | Begin_region | End_region | Specific _ | Dls_get | Poll | Alloc _
               )
-        | Reloadretaddr | Pushtrap _ | Poptrap | Prologue | Stack_check _ ->
+        | Reloadretaddr | Pushtrap _ | Poptrap _ | Prologue | Stack_check _ ->
           let is_op_end_region = function[@ocaml.warning "-4"]
             | Cfg.(Op End_region) -> true
             | _ -> false

backend/cfg/cfg_comballoc.ml

@@ -39,7 +39,7 @@ let rec find_next_allocation : cell option -> allocation option =
         | Reinterpret_cast _ | Static_cast _ | Probe_is_enabled _ | Opaque
         | Begin_region | End_region | Specific _ | Name_for_debugger _ | Dls_get
         | Poll )
-    | Reloadretaddr | Pushtrap _ | Poptrap | Prologue | Stack_check _ ->
+    | Reloadretaddr | Pushtrap _ | Poptrap _ | Prologue | Stack_check _ ->
       find_next_allocation (DLL.next cell))
 
 (* [find_compatible_allocations cell ~curr_mode ~curr_size] returns the
@@ -80,7 +80,7 @@ let find_compatible_allocations :
         | Local -> return ()
         | Heap -> loop allocations (DLL.next cell) ~curr_mode ~curr_size)
       | Op Poll -> return ()
-      | Reloadretaddr | Poptrap | Prologue | Pushtrap _ | Stack_check _ ->
+      | Reloadretaddr | Poptrap _ | Prologue | Pushtrap _ | Stack_check _ ->
         (* CR-soon xclerc for xclerc: is it too conservative? (note: only the
            `Pushtrap` case may be too conservative) *)
         { allocations = List.rev allocations; next_cell = Some cell }

backend/cfg/cfg_cse.ml

@@ -347,7 +347,7 @@ module Cse_generic (Target : Cfg_cse_target_intf.S) = struct
    fun state n cell ->
     let i = DLL.value cell in
     match i.desc with
-    | Reloadretaddr | Pushtrap _ | Poptrap | Prologue | Stack_check _ -> n
+    | Reloadretaddr | Pushtrap _ | Poptrap _ | Prologue | Stack_check _ -> n
     | Op (Move | Spill | Reload) ->
       (* For moves, we associate the same value number to the result reg as to
          the argument reg. *)

backend/cfg/cfg_deadcode.ml

@@ -22,7 +22,7 @@ let remove_deadcode (body : Cfg.basic_instruction_list) changed liveness
         match instr.desc with
         | Op _ as op ->
           Cfg.is_pure_basic op && Reg.disjoint_set_array !used_after instr.res
-        | Reloadretaddr | Pushtrap _ | Poptrap | Prologue | Stack_check _ ->
+        | Reloadretaddr | Pushtrap _ | Poptrap _ | Prologue | Stack_check _ ->
           false
       in
       used_after := before;