Liveness: improve for register allocation

This somehow reverts commit 72d20a6

Co-authored-by: Benjamin Grēgoire <Benjamin.Gregoire@inria.fr>

CHANGELOG.md

@@ -47,6 +47,10 @@
   argument (the shift amount) is no longer truncated
   ([PR #413](https://github.com/jasmin-lang/jasmin/pull/413)).
 
+- Improve liveness analysis for register allocation
+  ([PR #469](https://github.com/jasmin-lang/jasmin/pull/469);
+  fixes [#455](https://github.com/jasmin-lang/jasmin/issues/455)).
+
 ## Other changes
 
 - Explicit if-then-else in flag combinations is no longer supported

compiler/safetylib/domains/safetyProduct.ml

@@ -487,7 +487,7 @@ module PIMake (PW : ProgWrap) : VDomWrap = struct
   (* We compute the dependency heuristic graph *)
   let pa_res = Pa.pa_make PW.main (Some PW.prog)
 
-  (* We compute the reflexive and transitive clojure of dp *)
+  (* We compute the reflexive and transitive closure of dp *)
   let dp = trans_closure pa_res.pa_dp
 
   (* We are relational on a variable v iff:
@@ -578,11 +578,8 @@ module PIDynMake (PW : ProgWrap) : VDomWrap = struct
      must be uniquely characterized by their names. *)
   let ssa_main, pa_res =
     (* FIXME: code duplication! dirty hack *)
-    let is_move_op =
-      X86_arch_full.X86_core.aparams.ap_is_move_op
-    in
     let asmOp = Arch_extra.asm_opI X86_arch_full.X86_core.asm_e in
-    FSPa.fs_pa_make asmOp is_move_op PW.main
+    FSPa.fs_pa_make asmOp PW.main
 
   (* We compute the reflexive and transitive clojure of dp *)
   let dp = trans_closure pa_res.pa_dp

compiler/safetylib/safetyPreanalysis.ml

@@ -473,7 +473,7 @@ end
 
 (* Flow-sensitive Pre-Analysis *)
 module FSPa : sig    
-  val fs_pa_make : X86_extra.x86_extended_op Sopn.asmOp -> (X86_extra.x86_extended_op -> bool) -> ('info, X86_extra.x86_extended_op) func -> (unit, X86_extra.x86_extended_op) func * Pa.pa_res
+  val fs_pa_make : X86_extra.x86_extended_op Sopn.asmOp -> ('info, X86_extra.x86_extended_op) func -> (unit, X86_extra.x86_extended_op) func * Pa.pa_res
 end = struct
   exception Fcall
   let rec collect_vars_e sv = function
@@ -525,7 +525,7 @@ end = struct
     Sv.for_all (fun v -> not (Sv.exists (fun v' ->
         v.v_id <> v'.v_id && v.v_name = v'.v_name) sv)) sv
 
-  let fs_pa_make asmOp is_move_op (f : ('info, 'asm) func) =
+  let fs_pa_make asmOp (f : ('info, 'asm) func) =
     let sv = Sv.of_list f.f_args in
     let vars = try collect_vars_is sv f.f_body with
       | Fcall ->
@@ -536,7 +536,7 @@ end = struct
     (* We make sure that variable are uniquely defined by their names. *)
     assert (check_uniq_names vars);
 
-    let ssa_f = Ssa.split_live_ranges is_move_op false f in
+    let ssa_f = Ssa.split_live_ranges false f in
     debug (fun () ->
         Format.eprintf "SSA transform of %s:@;%a"
           f.f_name.fn_name

compiler/safetylib/safetyPreanalysis.mli

@@ -27,7 +27,6 @@ end
 module FSPa : sig
   val fs_pa_make :
     X86_extra.x86_extended_op Sopn.asmOp ->
-    (X86_extra.x86_extended_op -> bool) ->
     ('info, X86_extra.x86_extended_op) func -> (unit, X86_extra.x86_extended_op) func * Pa.pa_res
 end
 

compiler/src/compile.ml

@@ -163,7 +163,7 @@ let compile (type reg regx xreg rflag cond asm_op extra_op)
 
   let removereturn sp =
     let fds, _data = Conv.prog_of_csprog sp in
-    let tokeep = RemoveUnusedResults.analyse Arch.aparams.ap_is_move_op fds in
+    let tokeep = RemoveUnusedResults.analyse fds in
     tokeep
   in
 

compiler/src/liveness.ml

@@ -19,43 +19,32 @@ let writev_lval s = function
 
 let writev_lvals s lvs = List.fold_left writev_lval s lvs
 
-let is_trivial_move x e =
-  match x, e with
-  | Lvar x, Pvar y -> is_gkvar y && kind_i x = kind_i y.gv
-  | _              -> false
-
-(* Extends [is_move_op] to [asm_op Sopn.sopn] *)
-let is_move_sopn is_move_op o =
-  match o with
-  | Sopn.Oasm o -> is_move_op o
-  | _ -> false
-
 (* When [weak] is true, the out live-set contains also the written variables. *)
-let rec live_i is_move_op weak i s_o =
-  let s_i, s_o, d = live_d is_move_op weak i.i_desc s_o in
+let rec live_i weak i s_o =
+  let s_i, s_o, d = live_d weak i.i_desc s_o in
   s_i, { i_desc = d; i_loc = i.i_loc; i_info = (s_i, s_o); i_annot = []}
 
-and live_d is_move_op weak d (s_o: Sv.t) =
+and live_d weak d (s_o: Sv.t) =
   match d with
   | Cassgn(x, tg, ty, e) ->
 
     let s_i = Sv.union (vars_e e) (dep_lv s_o x) in
     let s_o =
-      if weak && not (is_trivial_move x e) then writev_lval s_o x
+      if weak then writev_lval s_o x
       else s_o in
     s_i, s_o, Cassgn(x, tg, ty, e)
 
   | Copn(xs,t,o,es) ->
     let s_i = Sv.union (vars_es es) (dep_lvs s_o xs) in
     let s_o =
-     if weak && not (is_move_sopn is_move_op o && is_trivial_move (List.hd xs) (List.hd es))
+     if weak
      then writev_lvals s_o xs
      else s_o in
     s_i, s_o, Copn(xs,t,o,es)
 
   | Cif(e,c1,c2) ->
-    let s1, c1 = live_c is_move_op weak c1 s_o in
-    let s2, c2 = live_c is_move_op weak c2 s_o in
+    let s1, c1 = live_c weak c1 s_o in
+    let s2, c2 = live_c weak c2 s_o in
     Sv.union (vars_e e) (Sv.union s1 s2), s_o, Cif(e, c1, c2)
 
   | Cfor _ -> assert false (* Should have been removed before *)
@@ -65,8 +54,8 @@ and live_d is_move_op weak d (s_o: Sv.t) =
     let rec loop s_o =
       (* loop (c; if e then c' else exit) *)
       let s_o' = Sv.union ve s_o in
-      let s_i, c = live_c is_move_op weak c s_o' in
-      let s_i', c' = live_c is_move_op weak c' s_i in
+      let s_i, c = live_c weak c s_o' in
+      let s_i', c' = live_c weak c' s_i in
       if Sv.subset s_i' s_o then s_i, (c,c')
       else loop (Sv.union s_i' s_o) in
     let s_i, (c,c') = loop s_o in
@@ -80,21 +69,21 @@ and live_d is_move_op weak d (s_o: Sv.t) =
     let s_i = Sv.union (vars_es es) (dep_lvs s_o xs) in
     s_i, (if weak then writev_lvals s_o xs else s_o), Csyscall(xs,o,es)
 
-and live_c is_move_op weak c s_o =
+and live_c weak c s_o =
   List.fold_right
     (fun i (s_o, c) ->
-      let s_i, i = live_i is_move_op weak i s_o in
+      let s_i, i = live_i weak i s_o in
       (s_i, i :: c))
     c
     (s_o, [])
 
-let live_fd is_move_op weak fd =
+let live_fd weak fd =
   let s_o = Sv.of_list (List.map L.unloc fd.f_ret) in
-  let _, c = live_c is_move_op weak fd.f_body s_o in
+  let _, c = live_c weak fd.f_body s_o in
   { fd with f_body = c }
 
-let liveness is_move_op weak prog = 
-  let fds = List.map (live_fd is_move_op weak) (snd prog) in
+let liveness weak prog =
+  let fds = List.map (live_fd weak) (snd prog) in
   fst prog, fds
 
 let iter_call_sites (cbf: L.i_loc -> funname -> lvals -> Sv.t * Sv.t -> unit)

compiler/src/liveness.mli

@@ -8,9 +8,9 @@ open Prog
  *)
 val dep_lvs : Sv.t -> lval list -> Sv.t
 
-val live_fd : ('asm -> bool) -> bool -> ('info, 'asm) func -> (Sv.t * Sv.t, 'asm) func
+val live_fd : bool -> ('info, 'asm) func -> (Sv.t * Sv.t, 'asm) func
 
-val liveness : ('asm -> bool) -> bool -> ('info, 'asm) prog -> (Sv.t * Sv.t, 'asm) prog
+val liveness : bool -> ('info, 'asm) prog -> (Sv.t * Sv.t, 'asm) prog
 
 (** [iter_call_sites cb f] runs the [cb] function for all call site in [f] with
       the location of the call instruction, the name of the called function, the

compiler/src/regalloc.ml

@@ -792,7 +792,7 @@ let reverse_varmap nv (vars: int Hv.t) : A.allocation =
   a
 
 let split_live_ranges (f: ('info, 'asm) func) : (unit, 'asm) func =
-  Ssa.split_live_ranges Arch.aparams.ap_is_move_op true f
+  Ssa.split_live_ranges true f
 
 let renaming (f: ('info, 'asm) func) : (unit, 'asm) func =
   let vars, nv = collect_variables ~allvars:true Sv.empty f in
@@ -872,8 +872,8 @@ let global_allocation translate_var (funcs: ('info, 'asm) func list) : (unit, 'a
   let killed fn = Hf.find killed_map fn in
   let preprocess f =
     Hf.add annot_table f.f_name f.f_annot;
-    let f = f |> fill_in_missing_names |> Ssa.split_live_ranges Arch.aparams.ap_is_move_op false in
-    Hf.add liveness_table f.f_name (Liveness.live_fd Arch.aparams.ap_is_move_op true f);
+    let f = f |> fill_in_missing_names |> Ssa.split_live_ranges false in
+    Hf.add liveness_table f.f_name (Liveness.live_fd true f);
     (* compute where will be store the return address *)
     let ra = 
        match f.f_cc with

compiler/src/removeUnusedResults.ml

@@ -10,9 +10,9 @@ let used_results (live: Sv.t) : lvals -> Sint.t =
     )
     Sint.empty
 
-let analyse is_move_op funcs =
+let analyse funcs =
   let liveness_table : (Sv.t * Sv.t, 'asm) func Hf.t = Hf.create 17 in
-  List.iter (fun (_,f) -> Hf.add liveness_table f.f_name (Liveness.live_fd is_move_op false f)) funcs;
+  List.iter (fun (_,f) -> Hf.add liveness_table f.f_name (Liveness.live_fd false f)) funcs;
   let live_results =
     let live : Sint.t Hf.t = Hf.create 17 in
     let cbf _loc fn xs (_, s) =

compiler/src/removeUnusedResults.mli

@@ -8,6 +8,4 @@ returned values that are never used by the callers.
 FIXME: this assumes that the program never calls export functions.
 
 *)
-val analyse : ('asm -> bool) ->
-              ('a * ('info, 'asm) func) list -> (funname -> bool list option)
-
+val analyse : ('a * ('info, 'asm) func) list -> funname -> bool list option

compiler/src/ssa.ml

@@ -52,8 +52,8 @@ let ir (m: names) (x: var) (y: var) : (unit, 'asm) instr =
   let i_desc = Cassgn (Lvar (v y), AT_phinode, y.v_ty, Pvar (gkvar (v x))) in
   { i_desc ; i_info = () ; i_loc = L.i_dummy ; i_annot = [] }
 
-let split_live_ranges is_move_op (allvars: bool) (f: ('info, 'asm) func) : (unit, 'asm) func =
-  let f = Liveness.live_fd is_move_op false f in
+let split_live_ranges (allvars: bool) (f: ('info, 'asm) func) : (unit, 'asm) func =
+  let f = Liveness.live_fd false f in
   let rec instr_r (li: Sv.t) (lo: Sv.t) (m: names) =
     function
     | Cassgn (x, tg, ty, e) ->

compiler/src/ssa.mli

@@ -1,7 +1,5 @@
 open Prog
 
-val split_live_ranges :
-  ('asm -> bool) ->
-  bool -> ('info, 'asm) func -> (unit, 'asm) func
+val split_live_ranges : bool -> ('info, 'asm) func -> (unit, 'asm) func
 
 val remove_phi_nodes : ('info, 'asm) func -> ('info, 'asm) func

compiler/src/stackAlloc.ml

@@ -180,7 +180,7 @@ let memory_analysis pp_err ~debug up =
       (Printer.pp_prog ~debug:true Arch.asmOp) ([], (List.map snd fds));
 
   (* remove unused result *)
-  let tokeep = RemoveUnusedResults.analyse Arch.aparams.ap_is_move_op fds in
+  let tokeep = RemoveUnusedResults.analyse fds in
   let tokeep fn = tokeep fn in
   let deadcode (extra, fd) =
     let (fn, cfd) = Conv.cufdef_of_fdef fd in

compiler/src/varalloc.ml

@@ -348,7 +348,7 @@ let alloc_stack_fd callstyle pd is_move_op get_info gtbl fd =
       classes Sv.empty in
   Mv.iter (check_class fd.f_name.fn_name fd.f_loc ptr_classes ptr_args) classes;
 
-  let fd = Live.live_fd is_move_op false fd in
+  let fd = Live.live_fd false fd in
   let (_, ranges), stack_pointers =
     live_ranges_stmt pd alias ptr_classes (0, Mint.empty) fd.f_body in
 

compiler/tests/success/x86-64/bug_455.jazz

@@ -0,0 +1,11 @@
+export
+fn main(reg u64 y) {
+    reg u64 x;
+
+    // Commenting either of the following two lines makes compilation succeed.
+    x = #MOV(y);  // Using x = y; makes compilation succeed.
+    x = 1;
+
+    x = y if x == 0;
+    [x] = x;
+}