[4.13] ocamlopt fails with "Fatal error: Liveness.fundecl"

[kit-ty-kate]

Trying to install owl-base.1.0.1 using the latest OCaml 4.13 (7a182bd) results in the following fatal error:

# (cd _build/default && /home/opam/.opam/4.13.0+trunk/bin/ocamlopt.opt -w -40 -g -I src/base/.owl_base.objs/byte -I src/base/.owl_base.objs/native -intf-suffix .ml -no-alias-deps -o src/base/.owl_base.objs/native/owl_maths_quadrature.cmx -c -impl src/base/owl_maths_quadrature.ml)
# >> Fatal error: Liveness.fundecl:
#                 pp/65
# Fatal error: exception Misc.Fatal_error

Looking at the commits since the last check, I suspect it might be an issue in #10039 (backported in the 4.13 branch)

[xavierleroy]

Was the last check before or after #10404 was merged? (commit 5e45b2e) Because it also affects liveness analysis.

[xavierleroy]

It looks like the Spilling phase produced wrong code on function camlOwl_maths_quadrature__gauss_legendre_inner_658 from src/base/maths/owl_maths_quadrature.ml:98,19-1142. However the failure prevents the printing of the code after spilling... Time for more debug printing!

[xavierleroy]

Was the last check before or after #10404 was merged? (commit 5e45b2e) Because it also affects liveness analysis.

Actually the culprit could also be #10414 (9b887df) because it touches spilling and reloading.

[kit-ty-kate]

opam-health-check runs every 3 or 4 days on 4.12 and 4.13. The last one used edbeb95 (just before #10404 was merged) without encountering any new issues.
EDIT: I meant to say #10039 not 10404

[Octachron]

Both #10404 and #10414 are anterior to the branching of 4.13 (and thus edbeb95).

[lthls]

I'm not sure I'll have time to debug this before going on holidays, but it seems that Deadcode removes the initialisation of pp (among other things). Given that the Safepoints PR did not update the deadcode module, I suspect that it is Liveness that computes incorrect information, and that leads Deadcode to assume some registers are not used even though they are.
Spill correctly notices that the register is actually used and inserts a spill instruction, even though the register hasn't been initialised, and Liveness notices that this is wrong and errors out.
Hopefully someone else will figure out what's going on from there.

[xavierleroy]

I'm on it, helped with copious printing code.

The gist of the bug is that the newly-inserted polling points force spilling of the pp variable, which is OK. However, the spill code insertion pass miscomputes the places where pp needs to be spilled. This could be related to #10414...

[xavierleroy]

Actually, @lthls was mostly right. Spill code insertion correctly expects pp to be initialized before the loops. Yet, dead code elimination correctly removed the initialization of pp, because it's a reference that is always assigned to before being used...

So, where's the bug? Dead code elimination runs before the polling insertion pass, and spill code insertion runs after. Inserting polling points adds new execution paths, since a poll can raise an exception! So it's wrong to eliminate dead code before inserting poll points.

I'll fix this shortly, but right now I need a drink or three to forget these horrors.

For reference, the Owl code that exhibits the error is of the following shape:

  let pp = ref dummy_value in
  begin try
    while true do
      ...
      for j = 1 to n do ... done;
      pp := ...;
      assert (not exit condition)
    done
  with _ -> ()
  end;
  use !pp

The poll point inserted in the inner for j loop makes it possible to reach the use of !pp without having assigned to pp.

Note the "elegance" of exiting a loop with a failed assertion. The imagination of our users has no limits.

Note also that the original code is now wrong if an exception handler is installed and can raise an exception. try ... with _ -> ... is almost always wrong, but after poll point insertion it's even more wrong.