Comment author: @alainfrisch

How often does this happen in practice? I've attached a patch which should handle this specific case, but one could clearly do a much better job at optimizing the %ignore primitive (removing all pure computations).

Have you noticed other similar cases not related to %ignore?

Comment author: @diml

The place where we noticed it originally was bin_prot. We have a set of combinators for computing the serialized size of a value, and the one for floats simply ignores its argument:

https://github.com/janestreet/bin_prot/blob/master/lib/size.ml#L55

Before the hack it was just: let bin_size_float _ = 8

Comment author: @alainfrisch

Thanks for the info. It's always useful when opening such a ticket to mention some real world case where it matters.

I've uploaded an improved patch which also removes the memory load in that case. The problem is that I don't fully understand the "semantics" of Cmm.remove_unit. It seems to me that it is always used in contexts where we don't care about the result, only the side effects of the computation. But then it could be much more aggressive and avoid for instance the default case (c -> Csequence (c, Ctuple [])) (and use c -> c instead). Perhaps there are more invariants (such as not changing the "type" of the sub-expression), and I don't feel confident enough to merge the commit in its current form.

Comment author: @diml

I wonder if this could be done at the level of deadcode.ml. Currently it doesn't get rid of the allocation because of the test [Proc.op_is_pure] which is false for [Ialloc]. But intuitively I would say that it is safe to get rid of unused allocations. If [Ialloc] can be removed then I think it will get rid of the memory load automatically as well.

Comment author: @alainfrisch

I think it's not the Ialloc that should be declared as pure. Its result is used in init stores, so even if declared pure, it won't be discarded. It seems we should rather do something like declaring "Istore(_, _, false)" pure. Is that sound?

Comment author: @diml

BTW I just tried with flambda and the allocation is avoided in both [f1] and [f2], so we can just revisit this when flambda is merged.

Comment author: @damiendoligez

flambda is now merged and indeed it avoids allocation in both f1 and f2.