Overwriting primitives in lambda

[anfelor]

This adds the basic primitives needed to support overwriting to lambda.

• In the middle end, we call todo_overwrite_not_implemented.
• In value_recursion_compiler, we disallow overwriting
• In tmc we allow overwriting.
• In bytecode, we map reuses to fresh allocations.

[goldfirere]

As @anfelor and I discussed over Slack, I think it would be a significant win to change the representation here, including reset_field elements only when we are actually changing a field. (One way to understand this is the observation that 'a option list is isomorphic to (int * 'a) list, ignoring trailing Nones.) Not sure what the consumer of this is going to look like, so maybe we can do something even better than an int to count the missing fields, but storing an int is better than making a list with likely a lot of Nones in it.

[anfelor]

See bytegen.ml for a consumer of this representation

[anfelor]

Note that each None corresponds to an underscore in the source program (eg. overwrite x with (_,3)). We should thus expect the number of Nones to be somewhat small and should focus just on what the best representation is for code quality.

[mshinwell]

I would have thought the list representation is easier actually.

[anfelor]

Which list representation do you mean? The 'a option list in the PR or the (int * 'a) list proposed by Richard?

[goldfirere]

And I don't think the number of Nones will be all that small, in the case of record updates: {x with f} can have an arbitrarily large number of Nones.

[anfelor]

Since Mark seemed happy with the current design, I would propose we keep the representation as it is now.

[goldfirere]

But these new primitives can't allocate, can they? It's one of the reasons I don't think the mode is necessary.

[anfelor]

The mode will soon also contain uniqueness information that informs us whether the new block will itself be used uniquely. One worry then might be if the optimizer can lift out Preuseblocks that are unique? (In fact, it can't: it could only do that if the cell we reuse is statically known, but that cell will have been marked as unique and can not be lifted out). Furthermore, we sometimes handle Pmakeblock and Preuseblock similarly (eg. in tmc.ml) and it might be helpful if they have the same fields.

[goldfirere]

This line is in the function primitive_may_allocate, and returning Some mode makes it sound like it might allocate. (Perhaps my comment about the mode buried my main point.)

One worry: it does allocate in bytecode. Does that matter here?

[anfelor]

It would be sound to keep the Some mode here, since primitive_may_allocate is only used in transl_primitive where this value causes strictly more checks. Since we have decided to map Preuseblock to Pmakeblock in the backend for the moment, I think we should stick with this code for now. However, I will add a CR to make all of these return None once the backend supports actual overwriting. The bytecode interpreter does not use stack allocation so this check is irrelevant there.

[goldfirere]

It's not clear to me that args should be the same here for the two cases of is_reusing. That is, when is_reusing says to allocate, args must be all the fields of the constructor. But when is_reusing says to reuse, args is only the changed fields. But that's not what this code does.

[anfelor]

Yes, for the tmc analysis we only need to consider the changed fields, since only they can contain a recursive call. The same code works in both cases.

[goldfirere]

I would expect some tests for the treatment here -- even if the tests just print the lambda code and then fall over. Or at least a loud comment somewhere reminding us to test this thoroughly before release. A mistake here could lead to miscompilation.

[goldfirere]

This, too, should get a bunch of testing.

[anfelor]

I agree that the additions to tmc and bytegen should be tested before we merge this. How about we first merge #3089 and then add some code to the type-checker that maps the overwrites to these primitives? Then we can run a few simple programs (like the unique_rbtree or unique_sort from the old PR) to test this code.

[goldfirere]

How about we first merge #3089 and then add some code to the type-checker that maps the overwrites to these primitives? Then we can run a few simple programs (like the unique_rbtree or unique_sort from the old PR) to test this code.

OK. I'll focus there and then we can return to this one.

[mshinwell]

@chambart and I were just discussing that it's probably easier if these primitives return the block which has been updated, so that subsequent uses of the unique block use the new variable (bound to the result of such primitives). This may help produce a more straightforward handling of the primitives in Flambda 2, in particular to help enforce the necessary "barrier" behaviour.

[anfelor]

Oh, right, that is the intended meaning. If Preuseblock was a function, we should have: Preuseblock : memory cell -> overwritten args -> new block. You should not refer to the overwritten memory cell after it was passed to Preuseblock. In fact, referring to the overwritten memory cell again would break lots of properties since the overwrite may change its type.

[anfelor]

I have rebase the changes in this PR onto main and created a draft PR at #3350