search

ocaml-flambda / flambda-backend

The Flambda backend project for OCaml
94 stars 70 forks source link

Fix padding bug for float32 static values #2763

Closed mshinwell closed 2 weeks ago

mshinwell commented 3 weeks ago

The code in flambda2 to dynamically initialize static values containing float32s was correctly taking into account that they must be put into word-width slots. However the code for constant static fields was not, meaning that fields after a float32 would be at addresses 4 bytes too low.

The test case is in https://github.com/ocaml-flambda/flambda-backend/pull/2698. @ncik-roberts please add that as a test case in PR2698, since it needs inline mixed blocks.

ncik-roberts commented 2 weeks ago

The test case illustrated in the comments of https://github.com/ocaml-flambda/flambda-backend/pull/2698 doesn’t require inline record mixed blocks and can be copied here.

mshinwell commented 2 weeks ago

The test case illustrated in the comments of #2698 doesn’t require inline record mixed blocks and can be copied here.

Ah, indeed I see now. I've added it here.

mshinwell commented 2 weeks ago

This bug is probably present in https://github.com/ocaml-flambda/flambda-backend/pull/2712/ but I will fix that (and we can add some test cases there to form more robust checks, immune to closure offset computation changes).

ncik-roberts commented 2 weeks ago

Thanks. I’m planning on writing test cases for #2712 tomorrow.