[m-rev.] for post-commit review: compute NeedsUpdate directly

Peter Wang novalazy at gmail.com
Thu Mar 8 09:58:32 AEDT 2018


On Wed, 07 Mar 2018 20:04:58 +1100 (AEDT), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For post-commit review by Peter. I would like to hear whether
> you know the reason for the change of semantics mentioned
> in an XXX comment, and whether the other issues mentioned
> in that comment are known limitations or not.
> 
> An earlier version of this diff checked whether the new, direct
> algorithm computed the same answers as the old, indirect one;
> it did throughout a full bootcheck, including the pack_args_reuse
> test case.
> 

>       % If any argument packed into a word needs updating,
>       % the whole word needs updating.
>       % XXX This code changes the meaning of the third argument of
>       % cell_to_reuse, from having one element for each *argument*,
>       % to one element for each *word* or *double word*.
>       % I (zs) see two problems with this. First, the change in
>       % meaning is not reflected in the data structure anywhere,

Agreed.

>       % and second, given the potential presence of double-word floats,
>       % you cannot say simply that the Nth element of NeedsUpdates
>       % corresponds to the Nth word of the memory cell.
>       % Given that the different ConsIds may have double-word floats
>       % in different word positions, I don't see how a correctness
>       % argument for this code could be made.

I think this problem is averted by compute_reuse_type disabling reuse of
cells with different cons_ids.

Peter


More information about the reviews mailing list