CSE of involutions

[chambart]

This allows simplifying into the identity

let f x = not (not x)

[chambart]

The provided example is not completely simplified. The boxed integer and float cases do not trigger the CSE.
The problem is that the function looks like this:

    (λ〈k6〉《k7》(x/18 ∷ 𝕍=boxed_ℕ𝟞𝟜) (my_closure/19 ∷ 𝕍) .
     (let
      (prim/20 =
        ((Unbox_int64 x/18))
       prim/21 =
        ((~ prim/20))
       Pnegbint/22 =
        ((Box_int64 prim/21))
       prim/23 =
        ((Unbox_int64 Pnegbint/22))
       prim/24 =
        ((~ prim/23))
       Pnegbint/25 =
        ((Box_int64 prim/24))
       prim/26 =
        ((Unbox_int64 Pnegbint/25))
       prim/27 =
        ((~ prim/26))
       Pnegbint/28 =
        ((Box_int64 prim/27)))
      return k6 Pnegbint/28))))

When looking for the second negation in the CSE environment, it is looking for (assuming no renaming) (~ prim/23), while the environment contains (~ prim/21). The problem is that prim/23 is not the canonical alias, and Simplify_primitive.apply_cse tries it on the original primitive, before substituting the arguments for the canonical aliases, which here for prim/23 is prim/21

Should I fix it here ? I'm a bit worried that it might cause some other problems that I don't foresee. Otherwise, that might just be a new case to add to the missed_optimisation.md list

[mshinwell]

@chambart This seems like a bug we should fix, I think. The CSE code was supposed to be doing the appropriate canonicalisation to avoid this sort of problem. Can you have a look?

[chambart]

I added abefc4a to update the primitive with the simplified arguments before applying CSE.
This allocates a bit on almost all primitive simplifications. I could change the type of Flambda_primitive.update_args to avoid allocating the args list. I could also change Eligible_for_cse.create to directly take the list of new arguments. But both would make the code less readable.

Otherwise I added a test for involutions. It does not work right now because of the max inlining depth that prevents the involution function from being inlined. So it is here but I didn't activate it for the CI yet.

[Gbury]

Can't the test specify in its config a custom inlining depth to pass through to the ocamlopt cli ?

[lthls]

ocamlopt_flags = "-inline-max-depth 4" ?

[chambart]

It is disabled right now, so this will have to wait for the rec info things to be finished

ocaml/middle_end/flambda/inlining/inlining_state.ml

Lines 38 to 42 in 6e33b8f

	let is_depth_exceeded t =
	(* CR-soon lmaurer: Fix this once rec_info is functional again; hardcoding
	depth of 1 until then *)
	if true then t.depth >= 1 else
	t.depth >= (Inlining_arguments.max_inlining_depth t.arguments)

[mshinwell]

Hmm. Don't we want to rebind original_prim to the version with the simplified arguments? Otherwise, further down, in the None case we'll end up storing the primitive in the environment with the un-simplified arguments.

[mshinwell]

This probably should be a fatal error, let's try.