From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11192 invoked by alias); 19 Jun 2017 09:16:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 11177 invoked by uid 89); 19 Jun 2017 09:16:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=RCODE, driven, artificially X-HELO: mail-ot0-f182.google.com Received: from mail-ot0-f182.google.com (HELO mail-ot0-f182.google.com) (74.125.82.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Jun 2017 09:16:10 +0000 Received: by mail-ot0-f182.google.com with SMTP id s7so63824881otb.3 for ; Mon, 19 Jun 2017 02:16:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=clMG/e/K5Jz5WPQ36ZaWfCzUBgEabxtk1qRiD+gpvjg=; b=ofbgB8/j9RbcnuX2MK/cTcvoX7PkTmo9o/aIqXXK4lYhVlpt2KBdVAOQNADQTrg1wo 2YpoNiq0LAyOP674BLO4wi27db8beiymYC6jbVjI6/QBFwZQCchyaIbgXK6RCzKpnGGD ifIzNICHGTRGR9zDgb8BMBoo271ukuL/1P+nuLMoZ3g7JfCmXj7dcjd8TPZ2hn7rM44c 2wSEeuJrORnsR7iBRque9e1uUIvEhy2r+IzxD5MCyAGiv31PK+7OyoMliOsuEKCpAbTE N3greKf3eMQIX3iKGxDws2GepON5NckIJdEpeWMgB6CeBPNQfm9zLZNr4ETWq9DzodMI ctWw== X-Gm-Message-State: AKS2vOx9kjVzLc3xOivGvdVMeJZR7ua/bUTvlR64W7rImxK/lbQg/WF+ 50KvjJGESXlmGNUaWmhqGxOBWtNfoA== X-Received: by 10.157.7.199 with SMTP id 65mr3822883oto.237.1497863773605; Mon, 19 Jun 2017 02:16:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.37.66 with HTTP; Mon, 19 Jun 2017 02:16:13 -0700 (PDT) In-Reply-To: References: From: Richard Biener Date: Mon, 19 Jun 2017 09:16:00 -0000 Message-ID: Subject: Re: Prevent infinite recursion between simplification and CSE in FRE To: Marc Glisse Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg01274.txt.bz2 On Sat, Jun 17, 2017 at 9:35 AM, Marc Glisse wrote: > Hello, > > see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80887#c10 for the context. > FRE can go into an infinite recursion with some match.pd simplifications > (that have been temporarily reverted). > > Limiting the depth of recursive calls is not a great solution, but it is > simple and I don't have a good idea for an alternate solution that does not > disable a lot of desirable optimizations. > > There are many ways to write the limiter, I went with > > depth_limiter d; > if (d > 100) return false; > > but I could also have written the class so the use would look like > > depth_limiter d(100); > if (!d) return false; > > for instance. > > 100 was picked arbitrarily, I don't think it is worth having a param for it, > but we could certainly use a different value. > > Bootstrap and testsuite on powerpc64le-unknown-linux-gnu. I looked into the PR and I can't see anything wrong with the sequence of events (they are just unfortunate...). Somehow it feels the fix should be somewhere in the used mprts_hook because w/o this hook we cannot run into this kind of recursion. We can (and do, there's still at least one open PR ...) run into oscillations between two simplifications and this also happens for GENERIC folding and the patch catches this case as well. The consequence of stopping the recursion at an arbitrary point is a missed optimization (in the PR there's no existing value we can value-number to, so for that specific case it doesn't matter -- maybe that's always the case with mprts_hook driven recursions). So the nice thing about the patch is that we catch all cases but the bad thing is that we don't anymore ICE on trivially contradicting patterns ... So the following is a SCCVN local recursion prevention - works on the testcase. Can you poke holes into it? Index: gcc/tree-ssa-sccvn.c =================================================================== --- gcc/tree-ssa-sccvn.c (revision 249358) +++ gcc/tree-ssa-sccvn.c (working copy) @@ -1648,8 +1648,21 @@ vn_lookup_simplify_result (code_helper r if (!rcode.is_tree_code ()) return NULL_TREE; vn_nary_op_t vnresult = NULL; - return vn_nary_op_lookup_pieces (TREE_CODE_LENGTH ((tree_code) rcode), - (tree_code) rcode, type, ops, &vnresult); + tree res = vn_nary_op_lookup_pieces (TREE_CODE_LENGTH ((tree_code) rcode), + (tree_code) rcode, type, ops, &vnresult); + /* We can end up endlessly recursing simplifications if the lookup above + presents us with a def-use chain that mirrors the original simplification. + See PR80887 for an example. Limit successful lookup artificially + to 10 times if we are called as mprts_hook. */ + if (res && mprts_hook) + { + static unsigned cnt; + if (cnt == 0) + cnt = 9; + else if (--cnt == 0) + mprts_hook = NULL; + } + return res; } /* Return a value-number for RCODE OPS... either by looking up an existing > 2017-06-19 Marc Glisse > > * gimple-match-head.c (depth_limiter): New class. > * genmatch.c (decision_tree::gen): Use it. > > -- > Marc Glisse