Closed mshinwell closed 2 months ago
@lthls is going to review this.
I've found a few bugs remaining in this PR that will need to be addressed. One is a variant of #2763 (To_cmm_static.static_field
needs to be updated so that all expressions have the correct size), and I also suspect a bug in the translation of constant mixed blocks, where the code in Closure_conversion
fails to properly unbox the constants that have kind Float_boxed
. Hopefully fixing both of those should be enough to make the testsuite go green.
This is based on #2692 although for review purposes, it should be read from
0d24f6a7bc9fff5bd14bccc1b97304c73b9474ed
as a whole. It should provide full support for mixed block static allocation in both classic and optimized modes.One nice thing is as follows: we can ditch the old
Field_of_static_block
type entirely and just use aSimple
paired with aDebuginfo
. In addition we can just have a single constructor for scannable blocks inStatic_const
.The types have been carefully chosen with regard to the
Scannable
submodules so the encoding should be precise. (Some of the block shape types introduced in #2533 have been reworked a little to make this all fit together nicely.) The block shapes being carried along with the lists ofSimple
s used to build a statically-allocated block provide all the necessary information required for compilation to Cmm.Since it was useful for getting the types right I've also added support in classic mode for statically allocating all-float blocks (note these are blocks, e.g. from records, not arrays so the float array optimization isn't relevant here: they are always stored unboxed). There are no approximations for these however.
In addition there is a commit which introduces a new type to describe elements of a mixed block's flat suffix. I think we should avoid hanging onto Lambda types into Flambda 2: separate types for separate IRs enable nuances to be encoded more effectively, amongst other things. Here there is a concrete example: the
Float_boxed
constructor doesn't seem to make sense at Cmm time in the existing Lambda type that is being used for flat suffix elements, since any boxing required would have been expanded on the way into Flambda 2.I need to do another pass to make sure everything looks ok and tests pass, but I'm putting this up now to gather preliminary feedback.