On 30/03/2016 01:06, Reed Wilson wrote:

Hi list,

I made a small function to demonstrate it:

let compare_str_sub p s s_off len =

if s_off < 0 || s_off + len > String.length s

then invalid_arg "Don't do that!";

else p = s

...

The odd code is toward the beginning: (if (!= 3 1) (exit 2) (exit 3))

I don't know a lot about cmm code, but it looks like something the compiler should be able to optimize better. Fiddling with the flambda optimization options doesn't seem to remove it.

Is this just due to how new flambda is, or is there some other reason that code makes it through?

Thanks,

Reed Wlison

This is some kind of code that was introduced by Cmmgen:

Here is the clambda generated by flambda

(if
(if (< s_off_7/1208 0) 1
    (let (Pintcomp_arg_15/1217 (string.length s_8/1209))
      (> (+ s_off_7/1208 len_6/1207) Pintcomp_arg_15/1217)))
(apply* camlPervasives__invalid_arg_279 "camlHop__apply_arg_31")
(caml_string_equal p_9/1210 s_8/1209))

and the one without

(if
(|| (< s_off/1206 0)
    (> (+ s_off/1206 len/1207) (string.length s/1205)))
(apply* camlPervasives__invalid_arg_1007
    "camlHop__1"="Don't do that!")
(caml_string_equal p/1204 s/1205))) ))

You will notice this is almost identical, but that the '||' operator is in the 'if then else' form for the flambda version. This is due to the early conversion of '||' and '&&' to simplify things in the middle end. The downside being that some specific patterns recognized by the cmm generation are not recognized anymore (hence generating stupid stuff). There was a pull request to add a few patterns to cmmgen to handle that case, but some part where apparently lost in a merge conflict: https://github.com/ocaml/ocaml/pull/430/commits/355cf1d40b854711911ed332e9472cbd231ffc78

Thanks for the report !

For this kind of things, you should also open a ticket on mantis to keep track of it. I will soon open a PR to fix this.
--
Pierre