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

Ensure that comballoc.ml does not reverse the order of allocations #1917

Merged
merged 2 commits into from Nov 14, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changes
Expand Up @@ -299,6 +299,9 @@ Working version
stubs on 32bits
(Jérémie Dimino)

- GPR#1917: comballoc: ensure object allocation order is preserved
(Stephen Dolan)

### Runtime system:

- MPR#7198, MPR#7750, GPR#1738: add a function (caml_custom_alloc_mem)
Expand Down
51 changes: 26 additions & 25 deletions asmcomp/comballoc.ml
Expand Up @@ -18,41 +18,42 @@
open Mach

type allocation_state =
No_alloc (* no allocation is pending *)
| Pending_alloc of Reg.t * int (* an allocation is pending *)
(* The arguments of Pending_alloc(reg, ofs) are:
reg the register holding the result of the last allocation
ofs the alloc position in the allocated block *)
No_alloc
| Pending_alloc of
{ reg: Reg.t; (* register holding the result of the last allocation *)
totalsz: int } (* amount to be allocated in this block *)

let allocated_size = function
No_alloc -> 0
| Pending_alloc(_, ofs) -> ofs
| Pending_alloc {totalsz; _} -> totalsz

let rec combine i allocstate =
match i.desc with
Iend | Ireturn | Iexit _ | Iraise _ ->
(i, allocated_size allocstate)
| Iop(Ialloc { bytes = sz; _ }) ->
begin match allocstate with
No_alloc ->
let (newnext, newsz) =
combine i.next (Pending_alloc(i.res.(0), sz)) in
(instr_cons_debug (Iop(Ialloc {bytes = newsz; spacetime_index = 0;
label_after_call_gc = None; }))
i.arg i.res i.dbg newnext, 0)
| Pending_alloc(reg, ofs) ->
if ofs + sz < Config.max_young_wosize * Arch.size_addr then begin
let (newnext, newsz) =
combine i.next (Pending_alloc(reg, ofs + sz)) in
(instr_cons (Iop(Iintop_imm(Iadd, ofs))) [| reg |] i.res newnext,
newsz)
end else begin
let (newnext, newsz) =
combine i.next (Pending_alloc(i.res.(0), sz)) in
(instr_cons_debug (Iop(Ialloc { bytes = newsz; spacetime_index = 0;
label_after_call_gc = None; }))
i.arg i.res i.dbg newnext, ofs)
end
| Pending_alloc {reg; totalsz}
when totalsz + sz < Config.max_young_wosize * Arch.size_addr ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Why is the multiplication by Arch.size_addr here? (Yes, I know it was there before...)

A: It looks like I messed up when adding the inline record field called words. It should be called bytes instead. I will make a pull request to fix this. (This shows the value of inline record fields rather than anonymous constructor arguments, especially when units of measure are concerned that do not have their own types.)

let (next, totalsz) =
combine i.next
(Pending_alloc { reg = i.res.(0); totalsz = totalsz + sz }) in
(instr_cons_debug (Iop(Iintop_imm(Iadd, -sz)))
[| reg |] i.res i.dbg next,
totalsz)
| No_alloc | Pending_alloc _ ->
let (next, totalsz) =
combine i.next
(Pending_alloc { reg = i.res.(0); totalsz = sz }) in
let next =
let offset = totalsz - sz in
if offset = 0 then next
else instr_cons_debug (Iop(Iintop_imm(Iadd, offset))) i.res
i.res i.dbg next
in
(instr_cons_debug (Iop(Ialloc {bytes = totalsz; spacetime_index = 0;
label_after_call_gc = None; }))
i.arg i.res i.dbg next, allocated_size allocstate)
end
| Iop(Icall_ind _ | Icall_imm _ | Iextcall _ |
Itailcall_ind _ | Itailcall_imm _) ->
Expand Down