Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Liveness: improve for register allocation #469

Merged
merged 2 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions compiler/safetylib/domains/safetyProduct.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 *)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so frequent that I am able to find a typo in a comment (clojure -> closure) :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

let dp = trans_closure pa_res.pa_dp
Expand Down
6 changes: 3 additions & 3 deletions compiler/safetylib/safetyPreanalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ->
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion compiler/safetylib/safetyPreanalysis.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/compile.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
41 changes: 15 additions & 26 deletions compiler/src/liveness.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 *)
Expand All @@ -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
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/liveness.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion compiler/src/printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ let pp_var ~debug =
else
fun fmt x -> F.fprintf fmt "%s" x.v_name

let pp_dvar ~debug fmt x =
let pp_dloc fmt d =
if not (L.isdummy d) then F.fprintf fmt " (defined at %a)" L.pp_loc d
in
F.fprintf fmt "%a%a" (pp_var ~debug) x pp_dloc x.v_dloc

let pp_expr ~debug fmt e =
let pp_var = pp_var ~debug in
pp_ge pp_len pp_var fmt e
Expand Down Expand Up @@ -409,7 +415,7 @@ let pp_warning_msg fmt = function
let pp_err ~debug fmt (pp_e : Compiler_util.pp_error) =
let pp_var fmt v =
let v = Conv.var_of_cvar v in
Format.fprintf fmt "%a (defined at %a)" (pp_var ~debug) v L.pp_loc v.v_dloc
Format.fprintf fmt "%a" (pp_dvar ~debug) v
in
let rec pp_err fmt pp_e =
match pp_e with
Expand Down
1 change: 1 addition & 0 deletions compiler/src/printer.mli
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ val pp_pprog : ('reg, 'regx, 'xreg, 'rflag, 'cond, 'asm_op, 'extra_op) Arch_extr
Format.formatter -> ('info, ('reg, 'regx, 'xreg, 'rflag, 'cond, 'asm_op, 'extra_op) Arch_extra.extended_op) pprog -> unit

val pp_var : debug:bool -> Format.formatter -> var -> unit
val pp_dvar : debug:bool -> Format.formatter -> var -> unit

val string_of_combine_flags : Expr.combine_flags -> string

Expand Down
10 changes: 5 additions & 5 deletions compiler/src/regalloc.ml
Original file line number Diff line number Diff line change
Expand Up @@ -762,8 +762,8 @@ let greedy_allocation
| Vector -> push_var vectors i v
| Flag -> push_var flags i v
| Unknown ty ->
hierror_reg ~loc:Lnone "unable to allocate variable %a (defined at %a): no register bank for type %a"
(Printer.pp_var ~debug:true) v L.pp_loc v.v_dloc PrintCommon.pp_ty ty
hierror_reg ~loc:Lnone "unable to allocate variable %a: no register bank for type %a"
(Printer.pp_dvar ~debug:true) v PrintCommon.pp_ty ty
) vars;
two_phase_coloring Arch.allocatable_vars scalars cnf fr a;
two_phase_coloring Arch.extra_allocatable_vars extra_scalars cnf fr a;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/removeUnusedResults.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/removeUnusedResults.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions compiler/src/ssa.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/ssa.mli
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion compiler/src/stackAlloc.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/varalloc.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions compiler/tests/success/x86-64/bug_455.jazz
Original file line number Diff line number Diff line change
@@ -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;
}