From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Original-To: caml-list@sympa.inria.fr Delivered-To: caml-list@sympa.inria.fr Received: from mail3-relais-sop.national.inria.fr (mail3-relais-sop.national.inria.fr [192.134.164.104]) by sympa.inria.fr (Postfix) with ESMTPS id 8A5907EE34 for ; Thu, 31 Mar 2016 17:57:32 +0200 (CEST) IronPort-PHdr: 9a23:RVClJx9EbRD1l/9uRHKM819IXTAuvvDOBiVQ1KB92+kcTK2v8tzYMVDF4r011RmSDdWdsKoP2raempujcFJDyK7JiGoFfp1IWk1NouQttCtkPvS4D1bmJuXhdS0wEZcKflZk+3amLRodQ56mNBXsq3G/pQQfBg/4fVIsYL+lSsiJ14/mi6ibwN76XUZhvHKFe7R8LRG7/036l/I9ps9cEJs30QbDuXBSeu5blitCLFOXmAvgtI/rpMYwuwwZgf8q9tZBXKPmZOx4COUAVHV1e1wysfXquAPZQEOk5nIZWGwMjlIcGwnI9gv3GJzxuy7wv/Bh8C6RNMjyVaozHz+l6vE4ZgXvjXI9OjM1uF7Wjtd3iuoPqx+tphh7yqbbZYiUMLx1eaaLLoBSfnZIQssED38JOYi7dYZaSrNZZes= Authentication-Results: mail3-smtp-sop.national.inria.fr; spf=None smtp.pra=cedilla@gmail.com; spf=Pass smtp.mailfrom=cedilla@gmail.com; spf=None smtp.helo=postmaster@mail-oi0-f52.google.com Received-SPF: None (mail3-smtp-sop.national.inria.fr: no sender authenticity information available from domain of cedilla@gmail.com) identity=pra; client-ip=209.85.218.52; receiver=mail3-smtp-sop.national.inria.fr; envelope-from="cedilla@gmail.com"; x-sender="cedilla@gmail.com"; x-conformance=sidf_compatible Received-SPF: Pass (mail3-smtp-sop.national.inria.fr: domain of cedilla@gmail.com designates 209.85.218.52 as permitted sender) identity=mailfrom; client-ip=209.85.218.52; receiver=mail3-smtp-sop.national.inria.fr; envelope-from="cedilla@gmail.com"; x-sender="cedilla@gmail.com"; x-conformance=sidf_compatible; x-record-type="v=spf1" Received-SPF: None (mail3-smtp-sop.national.inria.fr: no sender authenticity information available from domain of postmaster@mail-oi0-f52.google.com) identity=helo; client-ip=209.85.218.52; receiver=mail3-smtp-sop.national.inria.fr; envelope-from="cedilla@gmail.com"; x-sender="postmaster@mail-oi0-f52.google.com"; x-conformance=sidf_compatible X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0D9AQB8R/1WizTaVdFdhAd9BqZ7CAEBBoERh3uIaYIdgXEjhTA6AoE8BzkTAQEBAQEBAQERAQEBCAsLCR8xgi2CFAEBAQMBEhEdARsdAQMBCwYDAgsNKgICIgERAQUBHAYTCBqHbwEDCggOlAuPQYExPjGLNoFqgleHAQoZJw1RhBwBAQEBAQEBAQEBAQEBAQEBAQEBARABBQoFhUeFDoQsP4JTglYFjjuEVoRhhXOFXoI3gX03jFmHSYYPER6BDg8SAYI3HoFyHDAFiGgBAQE X-IPAS-Result: A0D9AQB8R/1WizTaVdFdhAd9BqZ7CAEBBoERh3uIaYIdgXEjhTA6AoE8BzkTAQEBAQEBAQERAQEBCAsLCR8xgi2CFAEBAQMBEhEdARsdAQMBCwYDAgsNKgICIgERAQUBHAYTCBqHbwEDCggOlAuPQYExPjGLNoFqgleHAQoZJw1RhBwBAQEBAQEBAQEBAQEBAQEBAQEBARABBQoFhUeFDoQsP4JTglYFjjuEVoRhhXOFXoI3gX03jFmHSYYPER6BDg8SAYI3HoFyHDAFiGgBAQE X-IronPort-AV: E=Sophos;i="5.24,422,1454972400"; d="scan'208,217";a="171905808" Received: from mail-oi0-f52.google.com ([209.85.218.52]) by mail3-smtp-sop.national.inria.fr with ESMTP/TLS/AES128-GCM-SHA256; 31 Mar 2016 17:57:31 +0200 Received: by mail-oi0-f52.google.com with SMTP id p188so33672592oih.2 for ; Thu, 31 Mar 2016 08:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=yJchlmHM0fUi6BrcCFUS+eBimxnGWWbCuy67N1Y9Jkc=; b=UNRSBJ8ltD4/MqfV809zdOIBBW8DyKOdENMtgXYc+MB+5L+DV1mbmx4VrNV2o8d7TP 4mMoyv5mLU3joKt6hFxzs3ou7H7k5Xb4K/o6cRI6Rm9nrKJlDyrY1QHJTCd/+eLf+DmZ yp7VUF/wItsH1Fh/R9+1Zi4UizRo4UVquq+ElDzvozR+b734IEr+ndyrla+Df+L7pw2A wH5r4/ET/6hNCfoaglw4DaHddg15HWlWg0PH7ahHEuz2W89Nj1lTsJQmXCdW0Nx27qE1 dj/Jd6cUIUYF3qEh5/GV1G+HeHpd5VmjcpNRoOSGeuzmItNBexOwmK5/PNawdebc1E3s sR+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=yJchlmHM0fUi6BrcCFUS+eBimxnGWWbCuy67N1Y9Jkc=; b=UFaWqAHPNNq1gJF0ersADTmGsi/eCRG7Nzi+JTzawD69Q131WlyzFs+D4X1INCTvEX OE0Wgje0raYFT6tp6tWgawRP00oqhD34pKverwQlrhy1VrAjWVCqckhBs/dFUR4g75uv CVPrUaq1PSTlex2/LFYo001nBManPB0ZWTqMiSlusQUIFHcVOytucm6M+5DGNKq9e8Zn XpUS37MnyELm5yjMzzVVHG8QHklvojpix1xrS9UA6tkS+PCOTBFznWz2htXfbUs6lUNP z9c2Aqmj8YT2fQvjP21gw5sjqYM6hI3WyKKnoObu5NwWBuRtA+RBq5n6K/N0l6ufUwi3 iIvw== X-Gm-Message-State: AD7BkJJT1+eUMHyYJOv7IkI9/S2uZF7cNelGsGFMaWb01v/Sz99GAwM6kbEpB5Zf0HFaDrl8fvQe9Kqhsnq/gw== MIME-Version: 1.0 X-Received: by 10.157.33.2 with SMTP id i2mr2254otb.138.1459439850062; Thu, 31 Mar 2016 08:57:30 -0700 (PDT) Received: by 10.76.107.48 with HTTP; Thu, 31 Mar 2016 08:57:29 -0700 (PDT) In-Reply-To: <56FBA198.9000804@ocamlpro.com> References: <56FBA198.9000804@ocamlpro.com> Date: Thu, 31 Mar 2016 08:57:29 -0700 Message-ID: From: Reed Wilson To: Pierre Chambart Cc: caml-list@inria.fr Content-Type: multipart/alternative; boundary=001a113d1538d58b28052f5a5165 X-Validation-by: cedilla@gmail.com Subject: Re: [Caml-list] flambda seemingly missing easy optimization --001a113d1538d58b28052f5a5165 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks for the explanation. I was avoiding flambda as soon as I found this since I thought it would be representative of the state of its optimization. Good to know that's not the case! I recompiled after adding that branch back to cmmgen.ml and the problem seems to have disappeared. Thanks again, Reed PS. I wasn't too sure if mantis was still the recommended way of reporting bugs now that github is up and running. On Wed, Mar 30, 2016 at 2:51 AM, Pierre Chambart < pierre.chambart@ocamlpro.com> wrote: > 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 =3D > if s_off < 0 || s_off + len > String.length s > then invalid_arg "Don't do that!"; > else p =3D s > > ... > > > The odd code is toward the beginning: (if (!=3D 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"=3D"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 generati= on > 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/355cf1d40b854711911ed332e= 9472cbd231ffc78 > > 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 > --=20 =C3=A7 --001a113d1538d58b28052f5a5165 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thanks for the explanation. I was avoiding flambda as soon= as I found this since I thought it would be representative of the state of= its optimization. Good to know that's not the case!

I recompiled after adding that branch back to cmmgen.ml and the problem seems to have disappeared.

Thanks again,
Reed

PS. I wasn= 9;t too sure if mantis was still the recommended way of reporting bugs now = that github is up and running.

On Wed, Mar 30, 2016 at 2:51 AM, Pierre Chambart <= span dir=3D"ltr"><pierre.chambart@ocamlpro.com> wrote:
=20=20 =20=20=20=20 =20=20
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 =3D
if s_off < 0 || s_off + len > String.length s
then invalid= _arg "Don't do that!";
else p =3D s

...

The odd code is toward the beginning: (if (!=3D 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
=C2=A0 (if (< s_off_7/1208 0) 1
=C2=A0=C2=A0=C2=A0 (let (Pintcomp_arg_15/1217 (string.length s_8/1209))=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (> (+ s_off_7/1208 len_6/1207) Pintco= mp_arg_15/1217)))
=C2=A0 (apply* camlPervasives__invalid_arg_279=C2=A0 "camlHop__app= ly_arg_31")
=C2=A0 (caml_string_equal p_9/1210 s_8/1209))

and the one without

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

You will notice this is almost identical, but that the '||' ope= rator is in the 'if then else' form for the flambda version. This is = due to the early conversion of '||' and '&&' to sim= plify 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



--
=C3=A7
--001a113d1538d58b28052f5a5165--