From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94398 invoked by alias); 5 Mar 2019 10:44:34 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 94098 invoked by uid 89); 5 Mar 2019 10:44:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=H*i:sk:2h9w@ma, H*f:sk:2h9w@ma, H*i:OG6DPB, H*f:OG6DPB X-HELO: mail-lf1-f66.google.com Received: from mail-lf1-f66.google.com (HELO mail-lf1-f66.google.com) (209.85.167.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Mar 2019 10:44:32 +0000 Received: by mail-lf1-f66.google.com with SMTP id z23so5848745lfe.0; Tue, 05 Mar 2019 02:44:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=CEYXgzx8qvcqGEWT5SlhPOZynsA7iikdufLcVKIs9uM=; b=L+9fj+sMsRnSvx3XNM5wZ7JQd1+Cto7URPGzUn8Fv6xdcJt2ZqPqc0EldQyPZg4kBa wg3V9mB7yJTE5eJBnqnjgVd3SnwV9ppWJ973QgeHV+WUhgSNFDAevgknGj23UlwqZwC9 4gnCkaIQ5yZ+Q4vkSC/DWHU60vxgykm4lashZ8AyK0ltKvEfXB8qfURR1fitaqd6IfLj OkIUqYQGgmVEpKJCunimBL8NddVzHox2f5ujMBI/VyhTss9ehyxQ3uYlp4iRhHA1iPYb FxXsMLTnIRjTDBtP8rJTwwoIcW/bG8Mn2Q/0flZ+1/J+HYx6W9t8XLmTxMn+IWtmN9VU g/7w== MIME-Version: 1.0 References: <933b52ac-d372-f9d9-792e-4166f35b41f5@redhat.com> <327DC916-C1B4-47F9-92AE-468236D32C1F@oracle.com> <81846359-36B4-46F1-9C5C-8E20F927FE9F@oracle.com> In-Reply-To: From: Richard Biener Date: Tue, 05 Mar 2019 10:44:00 -0000 Message-ID: Subject: Re: A bug in vrp_meet? To: Qing Zhao Cc: GCC Development , Jeff Law , gcc Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-03/txt/msg00029.txt.bz2 On Tue, Mar 5, 2019 at 10:48 AM Richard Biener wrote: > > On Mon, Mar 4, 2019 at 11:01 PM Qing Zhao wrote: > > > > Hi, Richard, > > > > > On Mar 4, 2019, at 5:45 AM, Richard Biener wrote: > > >> > > >> It looks like DOM fails to visit stmts generated by simplification. = Can you open a bug report with a testcase? > > >> > > >> > > >> The problem is, It took me quite some time in order to come up with = a small and independent testcase for this problem, > > >> a little bit change made the error disappear. > > >> > > >> do you have any suggestion on this? or can you give me some hint on= how to fix this in DOM? then I can try the fix on my side? > > > > > > I remember running into similar issues in the past where I tried to > > > extract temporary nonnull ranges from divisions. > > > I have there > > > > > > @@ -1436,11 +1436,16 @@ dom_opt_dom_walker::before_dom_children > > > m_avail_exprs_stack->pop_to_marker (); > > > > > > edge taken_edge =3D NULL; > > > - for (gsi =3D gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > > > - { > > > - evrp_range_analyzer.record_ranges_from_stmt (gsi_stmt (gsi), f= alse); > > > - taken_edge =3D this->optimize_stmt (bb, gsi); > > > - } > > > + gsi =3D gsi_start_bb (bb); > > > + if (!gsi_end_p (gsi)) > > > + while (1) > > > + { > > > + evrp_range_analyzer.record_def_ranges_from_stmt (gsi_stmt (gs= i), false); > > > + taken_edge =3D this->optimize_stmt (bb, &gsi); > > > + if (gsi_end_p (gsi)) > > > + break; > > > + evrp_range_analyzer.record_use_ranges_from_stmt (gsi_stmt (gs= i)); > > > + } > > > > > > /* Now prepare to process dominated blocks. */ > > > record_edge_info (bb); > > > > > > OTOH the issue in your case is that fold emits new stmts before gsi b= ut the > > > above loop will never look at them. See tree-ssa-forwprop.c for code= how > > > to deal with this (setting a pass-local flag on stmts visited and wal= king back > > > to unvisited, newly inserted ones). The fold_stmt interface could in= theory > > > also be extended to insert new stmts on a sequence passed to it so the > > > caller would be responsible for inserting them into the IL and could = then > > > more easily revisit them (but that's a bigger task). > > > > > > So, does the following help? > > > > Yes, this change fixed the error in my side, now, in the dumped file fo= r pass dom3: > > > > =3D=3D=3D=3D > > Visiting statement: > > i_49 =3D _98 > 0 ? k_105 : 0; > > Meeting > > [0, 65535] > > and > > [0, 0] > > to > > [0, 65535] > > Intersecting > > [0, 65535] > > and > > [0, 65535] > > to > > [0, 65535] > > Optimizing statement i_49 =3D _98 > 0 ? k_105 : 0; > > Replaced 'k_105' with variable '_98' > > gimple_simplified to _152 =3D MAX_EXPR <_98, 0>; > > i_49 =3D _152; > > Ah, that looks interesting. From this detail we might be > able to derive a testcase as well - a GIMPLE one > eventually because DOM runs quite late. It's also interesting > to see the inefficient code here (the extra copy), probably > some known issue with match-and-simplify, I'd have to check. > > > Folded to: i_49 =3D _152; > > LKUP STMT i_49 =3D _152 > > =3D=3D=3D=3D ASGN i_49 =3D _152 > > > > Visiting statement: > > _152 =3D MAX_EXPR <_98, 0>; > > > > Visiting statement: > > i_49 =3D _152; > > Intersecting > > [0, 65535] EQUIVALENCES: { _152 } (1 elements) > > and > > [0, 65535] > > to > > [0, 65535] EQUIVALENCES: { _152 } (1 elements) > > =3D=3D=3D=3D > > > > We can clearly see from the above, all the new stmts generated by fold = are visited now. > > We can also see that DOMs optimize_stmt code is not executed on the first= stmt > of the folding result (the MAX_EXPR), so the fix can be probably > amended/simplified > with that in mind. > > > it is also confirmed that the runtime error caused by this bug was gone= with this fix. > > > > So, what=E2=80=99s the next step for this issue? > > > > will you commit this fix to gcc9 and gcc8 (we need it in gcc8)? > > I'll see to carve out some cycles trying to find a testcase and amend > the fix a bit > and will take care of testing/submitting the fix. Thanks for testing > that it works > for your case. I filed PR89595 with a testcase. Richard. > Richard. > > > or I can test this fix on my side and commit it to both gcc9 and gcc8? > > > > thanks. > > > > Qing > > > > > > > > Index: gcc/tree-ssa-dom.c > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > --- gcc/tree-ssa-dom.c (revision 269361) > > > +++ gcc/tree-ssa-dom.c (working copy) > > > @@ -1482,8 +1482,25 @@ dom_opt_dom_walker::before_dom_children > > > edge taken_edge =3D NULL; > > > for (gsi =3D gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > > > { > > > + gimple_stmt_iterator pgsi =3D gsi; > > > + gsi_prev (&pgsi); > > > evrp_range_analyzer.record_ranges_from_stmt (gsi_stmt (gsi), fa= lse); > > > taken_edge =3D this->optimize_stmt (bb, gsi); > > > + gimple_stmt_iterator npgsi =3D gsi; > > > + gsi_prev (&npgsi); > > > + /* Walk new stmts eventually inserted by DOM. gsi_stmt (gsi) = itself > > > + while it may be changed should not have gotten a new definit= ion. */ > > > + if (gsi_stmt (pgsi) !=3D gsi_stmt (npgsi)) > > > + do > > > + { > > > + if (gsi_end_p (pgsi)) > > > + pgsi =3D gsi_start_bb (bb); > > > + else > > > + gsi_next (&pgsi); > > > + evrp_range_analyzer.record_ranges_from_stmt (gsi_stmt (pg= si), > > > + false); > > > + } > > > + while (gsi_stmt (pgsi) !=3D gsi_stmt (gsi)); > > > } > > > > > > /* Now prepare to process dominated blocks. */ > > > > > > > > > Richard. > > > > > >> Thanks a lot. > > >> > > >> Qing > > >> > > >> > > >> > > >> Richard. > > >> > > >> > >