Skip to content

Commit

Permalink
flambda-backend: Fix missing rewrite ids when unboxing detects invali…
Browse files Browse the repository at this point in the history
…d apply conts (#2756)

* Add tests

* More detailed error message

* Fix missing rewrite ids when unboxing detect invalid apply conts

In some cases (e.g. a do_not_unbox decision, followed by a decision that
does unbox, and that discovers that some apply conts are invalids), it
could happen that the set of rewrite ids known as being invalid was
dropped/reset to empty. That together with the caching of the extra args
computsion (done through the rewrite ids seen) meant that we could
"forget" some rewrite ids.

* Allow overlap of invalid rewrite id and extra_args

As the comment states, it can happen that, when adding a extra param and
args, there is an overlap between the domain of the extra_args map, and
the set of invalids (because of the way these are computed by the
unboxing code). This case is reasonable, so in such a case, we can allow
the invalid set to take precedenhce.

* Force tests to be compiled with O3

Although not strictly necessary for the tests to work, it's safer to
ensure that they are optimized as expected
  • Loading branch information
Gbury authored Jul 4, 2024
1 parent 1ab77ad commit e892bb8
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 0 deletions.
37 changes: 37 additions & 0 deletions testsuite/tests/flambda/unboxing_finds_invalid1.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
(* TEST *)

[@@@flambda_o3]

type _ foo =
| Int : int foo
| Float : float foo

type _ bar =
| I : int -> int bar
| F : float -> float bar

type t = T : 'a foo * 'a -> t

let[@inline never] bar b = b

(* In this test, `z` is not unboxed, but `x` is, and one of the calls to `foo`
(which are all continuations calls, givne the @local), can be found to be
invalid because of the unboxing (it is not found earlier because the `Int`
value is hidden thanks to `Sys.opaque_identity).
In an early version of invalids during unboxing, there was a bug where in
such cases, there would be missing cases in the extra arguments computed
by the unboxing. *)
let test f g =
let[@local] foo (type a) z (x : a bar) =
match x with
| I i -> z i
| F f -> z (int_of_float f)
in
let aux = Sys.opaque_identity Int in
let t : t = T (aux, 0) in
match t with
| T (Int, i) -> foo f (I i)
| T (Float, f) -> foo g (F f)


33 changes: 33 additions & 0 deletions testsuite/tests/flambda/unboxing_finds_invalid2.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
(* TEST *)

[@@@flambda_o3]

type _ foo =
| Int : int foo
| Float : float foo

type _ bar =
| I : int -> int bar
| F : float -> float bar

type t = T : 'a foo * 'a -> t

let[@inline never] bar b = b

(* Here, both `b` and `x` are unboxed, and in an early version of
invalids during unboxing, this results in an overlap of rewrite id
between extra args computed for `b` and the invalids (which were found
when computing the extra args for `x`). *)
let test () =
let[@local] foo (type a) b (x : a bar) =
match x with
| I i -> if b then i else 0
| F f -> if b then int_of_float f else 0
in
let aux = Sys.opaque_identity Int in
let t : t = T (aux, 0) in
match t with
| T (Int, i) -> foo true (I i)
| T (Float, f) -> foo false (F f)


0 comments on commit e892bb8

Please sign in to comment.