Skip to content

Commit

Permalink
optimize locals a bit more
Browse files Browse the repository at this point in the history
  • Loading branch information
@Simn
Simn committed Jan 17, 2024
1 parent 9bfb8d2 commit 28ecf77
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions src/compiler/hxb/hxbWriter.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,14 @@ type field_writer_context = {
t_pool : StringPool.t;
pos_writer : PosWriter.t;
mutable texpr_this : texpr option;
vars : (int,tvar) pool;
vars : (tvar * int) DynArray.t;
}

let create_field_writer_context pos_writer = {
t_pool = StringPool.create ();
pos_writer = pos_writer;
texpr_this = None;
vars = new pool;
vars = DynArray.create ();
}

type hxb_writer = {
Expand Down Expand Up @@ -1266,7 +1266,12 @@ module HxbWriter = struct

and write_texpr writer (fctx : field_writer_context) (e : texpr) =
let declare_var v =
Chunk.write_uleb128 writer.chunk (fctx.vars#add v.v_id v);
let index = DynArray.length fctx.vars in
DynArray.add fctx.vars (v,v.v_id);
(* Store local index in v_id so we find it easily for all the TLocal expressions.
This is set back by the var writer in start_texpr. *)
v.v_id <- index;
Chunk.write_uleb128 writer.chunk index;
Chunk.write_option writer.chunk v.v_extra (fun ve ->
Chunk.write_list writer.chunk ve.v_params (fun ttp ->
let index = writer.local_type_parameters#add ttp () in
Expand Down Expand Up @@ -1309,7 +1314,7 @@ module HxbWriter = struct
(* vars 20-29 *)
| TLocal v ->
Chunk.write_u8 writer.chunk 20;
Chunk.write_uleb128 writer.chunk (fctx.vars#get v.v_id)
Chunk.write_uleb128 writer.chunk v.v_id;
| TVar(v,None) ->
Chunk.write_u8 writer.chunk 21;
declare_var v;
Expand Down Expand Up @@ -1657,12 +1662,11 @@ module HxbWriter = struct
List.iter (fun bytes ->
Chunk.write_bytes writer.chunk (Bytes.unsafe_of_string bytes)
) items;

let items = fctx.vars#items in
Chunk.write_uleb128 writer.chunk (DynArray.length items);
DynArray.iter (fun v ->
Chunk.write_uleb128 writer.chunk (DynArray.length fctx.vars);
DynArray.iter (fun (v,v_id) ->
v.v_id <- v_id;
write_var writer fctx v;
) items;
) fctx.vars;
Chunk.export_data new_chunk writer.chunk;
restore(fun new_chunk -> new_chunk)
)
Expand Down

8 comments on commit 28ecf77

@Simn
Copy link
Member Author

@Simn Simn commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

@kLabz Could you review this? It somehow broke everything, but it works for me locally...

@kLabz
Copy link
Contributor

@kLabz kLabz commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

Seems like genjvm doesn't like v_id = 0 ?

@Simn
Copy link
Member Author

@Simn Simn commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

It shouldn't have such an ID though.

But I think I know what's going on here.

@kLabz
Copy link
Contributor

@kLabz kLabz commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

Why wouldn't it start at 0?

	let index = DynArray.length fctx.vars in
	DynArray.add fctx.vars (v,v.v_id);
	(* Store local index in v_id so we find it easily for all the TLocal expressions.
	   This is set back by the var writer in start_texpr. *)
	v.v_id <- index;

@kLabz
Copy link
Contributor

@kLabz kLabz commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

Outch btw..

Checking dump diff =====
Diff 1 -> 4
90601
Diff 1 -> 2
90601
Diff 2 -> 4
0

Will check when that happened...

@Simn
Copy link
Member Author

@Simn Simn commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

v_id is reset, comment tells you :P

@kLabz
Copy link
Contributor

@kLabz kLabz commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

That diff is entirely from this commit :)

@Simn
Copy link
Member Author

@Simn Simn commented on 28ecf77 Jan 17, 2024

Choose a reason for hiding this comment

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

I have to look at this again I guess, for now I have reverted the changes.

Please sign in to comment.