From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id C88D03858C54 for ; Fri, 2 Sep 2022 12:22:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C88D03858C54 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x632.google.com with SMTP id qh18so3516828ejb.7 for ; Fri, 02 Sep 2022 05:22:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date; bh=sDaWS26knH8qdY7qFnGgUwGcywZsGkmxKs3lwSQ5G5k=; b=l/V5zAhKAVu2DPeSQpXjJob/Vq+c0lar/ybxWr67qPBjI+Io78QfPbBn/BG+BhBEUw zUqMnvbJsd03ikz+JmVXEGFnRjZ9MJNqDWzyNW+YI7u9eBco/F0/ct8YVva3sztLwSva rx5fptCyFduN//zg0yC0rqHbaLOvs+vUILUf3yhK0VUldYXhPOspW52hi5+Fb66gIBVr NAQ3HhDFYA0xyxVOwfB62BFU9ZDAmrsdaC8P0/xhhfGv/fJpShRBzZBbiaIoghKpu91C kIEfIZObCoxQqkImQGUKzqtnJ/9MdvEMApa6reOm5fgUSMGkFbKlqXMdO/eDkujHB8SV JK1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date; bh=sDaWS26knH8qdY7qFnGgUwGcywZsGkmxKs3lwSQ5G5k=; b=T9yBpsUUG5PtDeT0WQ8NKjA+FfjGWY0Vl7k/01rlORDRhhgWCxl5/5S/wNmrp4Sl+5 MaTrzZdam1hEwcGER0O/dcqdZmbM+EBU6FS0awHID51yZo6xiNcyZ/4niWmfsBVaW6qZ xjLyyZswayQ+nhEZ4L1AlJbeGeEVDtQmciDPvDB9NtRdGo0O6gzRIKQb4CYv91mPKmnK AwEhy1UbY632VOFJPTzOJ+ihnGVPgqMKOqoUWxHWOK2P8bBVxIrKs/VFX3xxMBMoqe4G gJM+ZJgFvWZFPsXU1fXrZlV2IhaQwcxQzAJcTcF6yqMhkrBaP9X4GO9Knwp+k8IimeO2 sTrQ== X-Gm-Message-State: ACgBeo064kfBMUWeK2aWl3PwNW76eCFnuh+1eEeGNqgYCU5E53goZVaU Y8QSh3SHUj4Aic7lzxzMS4pI2dz2x/J225D8YyQ= X-Google-Smtp-Source: AA6agR6ltUywDiGcmcAFDHH5knozPd0fNhhFaHj5WLgIndm7Mqe2tUPpW/G+iHOWZLOUfY9aNVRs9Ei78Tmdl2aQBXQ= X-Received: by 2002:a17:907:9712:b0:731:67db:1b48 with SMTP id jg18-20020a170907971200b0073167db1b48mr27064090ejc.754.1662121344274; Fri, 02 Sep 2022 05:22:24 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Fri, 2 Sep 2022 14:22:10 +0200 Message-ID: Subject: Re: Surprising CFG construction with goto from then to else To: =?UTF-8?Q?J=C3=B8rgen_Kvalsvik?= Cc: GCC Development Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=1.0 required=5.0 tests=BAYES_50,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SCC_5_SHORT_WORD_LINES,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Sep 2, 2022 at 11:50 AM J=C3=B8rgen Kvalsvik wrote: > > > Hello, > > I played some more with odd programs and the effect on control flow > graph construction (as a part of condition coverage support [1]) and > came across this: > > int fn (int a, int b, int c) { > int x =3D 0; > if (a && b) { > if (c) { > goto a_; > } else { > x =3D a; > } > } else { > a_: > x =3D (a - b); > } > > return x; > } > > Run through gcov --conditions I get: > > 4: 5: if (a && b) { > condition outcomes covered 2/2 > condition outcomes covered 2/2 > 2: 6: if (c) { > condition outcomes covered 2/2 > > Which is clearly not correct. So I started digging into why and dump the > CFG as the coverage profiling sees it https://i.imgur.com/d0q72rA.png > [2]. I apologize for the labeling, but A2 =3D a, A3 =3D b, A5 =3D c and A= 9 the > else block. The problem, which is what confuses the algorithm, is that a > and b don't share A9 as a successor (on false) as I would expect. > > If I add some operation before the label the problem disappears and a > and b share false-destination again https://i.imgur.com/PSrfaLC.png [3]. > > } else { > x++; > a_: > x =3D (a - b); > } > > 4: 5: if (a && b) { > condition outcomes covered 4/4 > 2: 6: if (c) { > condition outcomes covered 2/2 > > > When dumping the cfg in the former case with -fdump-tree-cfg-graph I get > a CFG without the split destinations in a and b > https://i.imgur.com/05MCjzp.png [3]. I would assume from this that the > graph dump happens after _more_ CFG transformations than the branch > profiling. > > So my questions are: > > 1. Is the control flow graph expected to be constructed as such where a > and b don't share outcome, or is it to be considered a bug? > 2. If yes, would it be problematic to push the branch coverage and > condition profiling to a later stage where the cfg has been fixed? I would say you should only see more nodes merged. It's a bit hard to foll= ow what you say with the namings - I usually run cc1 in gdb, breaking at execute_build_cfg where you can do, after build_gimple_cfg finished (and before cleanup_tree_cfg ()) do a 'dot-fn' in gdb which produces a nice picture of the CFG and code with graphviz. It looks like I would have expected, in particular we do not force a new basic-block to be generated for a_: after the D.1991: artificial label we have for the else. That might be premature optimization for your case (but the cleanup_tree_cfg () would immediately do that as well). Richard. > Thanks, > J=C3=B8rgen > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598165.html > > [2] dot file: > > digraph { > A0 -> A2 > A2 -> A3 > A2 -> A8 > A3 -> A5 > A3 -> A4 > A8 -> A9 > A9 -> A10 > A10 -> A11 > A11 -> A1 > A5 -> A6 > A5 -> A7 > A4 -> A9 > A6 -> A9 > A7 -> A10 > } > > [3] dot file: > > digraph { > A0 -> A2 > A2 -> A3 > A2 -> A7 > A3 -> A4 > A3 -> A7 > A7 -> A8 > A8 -> A9 > A9 -> A10 > A10 -> A1 > A4 -> A5 > A4 -> A6 > A5 -> A8 > A6 -> A9 > } > > [3] dot file: > > > overlap=3Dfalse; > subgraph "cluster_fn" { > style=3D"dashed"; > color=3D"black"; > label=3D"fn ()"; > fn_0_basic_block_0 > [shape=3DMdiamond,style=3Dfilled,fillcolor=3Dwhite,label=3D"ENTRY"]; > > fn_0_basic_block_1 > [shape=3DMdiamond,style=3Dfilled,fillcolor=3Dwhite,label=3D"EXIT"]; > > fn_0_basic_block_2 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |x\ =3D\ 0;\l\ > |if\ (a\ !=3D\ 0)\l\ > \ \ goto\ \;\ [INV]\l\ > else\l\ > \ \ goto\ \;\ [INV]\l\ > }"]; > > fn_0_basic_block_3 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |if\ (b\ !=3D\ 0)\l\ > \ \ goto\ \;\ [INV]\l\ > else\l\ > \ \ goto\ \;\ [INV]\l\ > }"]; > > fn_0_basic_block_4 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |if\ (c\ !=3D\ 0)\l\ > \ \ goto\ \;\ [INV]\l\ > else\l\ > \ \ goto\ \;\ [INV]\l\ > }"]; > > fn_0_basic_block_5 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |//\ predicted\ unlikely\ by\ goto\ predictor.\l\ > goto\ \;\ [INV]\l\ > }"]; > > fn_0_basic_block_6 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |x\ =3D\ a;\l\ > goto\ \;\ [INV]\l\ > }"]; > > fn_0_basic_block_7 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |a_:\l\ > |x\ =3D\ a\ -\ b;\l\ > }"]; > > fn_0_basic_block_8 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |D.2398\ =3D\ x;\l\ > }"]; > > fn_0_basic_block_9 > [shape=3Drecord,style=3Dfilled,fillcolor=3Dlightgrey,label=3D"{\:= \l\ > |\:\l\ > |return\ D.2398;\l\ > }"]; > > fn_0_basic_block_0:s -> fn_0_basic_block_2:n > [style=3D"solid,bold",color=3Dblack,weight=3D100,constraint=3Dtrue]; > fn_0_basic_block_2:s -> fn_0_basic_block_3:n > [style=3D"solid,bold",color=3Dforestgreen,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_2:s -> fn_0_basic_block_7:n > [style=3D"solid,bold",color=3Ddarkorange,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_3:s -> fn_0_basic_block_4:n > [style=3D"solid,bold",color=3Dforestgreen,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_3:s -> fn_0_basic_block_7:n > [style=3D"solid,bold",color=3Ddarkorange,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_4:s -> fn_0_basic_block_5:n > [style=3D"solid,bold",color=3Dforestgreen,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_4:s -> fn_0_basic_block_6:n > [style=3D"solid,bold",color=3Ddarkorange,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_5:s -> fn_0_basic_block_7:n > [style=3D"solid,bold",color=3Dblack,weight=3D100,constraint=3Dtrue]; > fn_0_basic_block_6:s -> fn_0_basic_block_8:n > [style=3D"solid,bold",color=3Dblack,weight=3D100,constraint=3Dtrue]; > fn_0_basic_block_7:s -> fn_0_basic_block_8:n > [style=3D"solid,bold",color=3Dblack,weight=3D100,constraint=3Dtrue]; > fn_0_basic_block_8:s -> fn_0_basic_block_9:n > [style=3D"solid,bold",color=3Dblack,weight=3D100,constraint=3Dtrue]; > fn_0_basic_block_9:s -> fn_0_basic_block_1:n > [style=3D"solid,bold",color=3Dblack,weight=3D10,constraint=3Dtrue]; > fn_0_basic_block_0:s -> fn_0_basic_block_1:n > [style=3D"invis",constraint=3Dtrue]; > } > }