From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66350 invoked by alias); 2 Dec 2015 17:49:03 -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 66332 invoked by uid 89); 2 Dec 2015 17:49:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 02 Dec 2015 17:49:00 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 9B9ACC0B0205 for ; Wed, 2 Dec 2015 17:48:59 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-91.phx2.redhat.com [10.3.113.91]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tB2HmxV4024145; Wed, 2 Dec 2015 12:48:59 -0500 Subject: Re: [PR67335] drop dummy zero from reverse VTA ops, fix infinite recursion To: Alexandre Oliva , gcc-patches@gcc.gnu.org References: Cc: jakub@redhat.com From: Jeff Law Message-ID: <565F2F0B.7010306@redhat.com> Date: Wed, 02 Dec 2015 17:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg00326.txt.bz2 On 11/26/2015 04:45 PM, Alexandre Oliva wrote: > VTA's cselib expression hashing compares expressions with the same > hash before adding them to the hash table. When there is a collision > involving a self-referencing expression, we could get infinite > recursion, in spite of the cycle breakers already in place. The > problem is currently latent in the trunk, because by chance we don't > get a collision. > > Such value cycles are often introduced by reverse_op; most often, > they're indirect, and then value canonicalization takes care of the > cycle, but if the reverse operation simplifies to the original value, > we used to issue a (plus V (const_int 0)), because at some point > adding a plain value V to a location list as a reverse_op equivalence > caused other problems. > > (Jakub, do you by any chance still remember what those problems were, > some 5+ years ago?) > > This dummy zero, in turn, caused the value canonicalizer to not fully > realize the equivalence, leading to more complex graphs and, > occasionally, to infinite recursion when comparing such > value-plus-zero expressions recursively. > > Simply using V solves the infinite recursion from the PR testcase, > since the extra equivalence and the preexisting value canonicalization > together prevent recursion while the unrecognized equivalence > wouldn't, but it exposed another infinite recursion in > memrefs_conflict_p: get_addr had a cycle breaker in place, to skip RTL > referencing values introduced after the one we're examining, but it > wouldn't break the cycle if the value itself appeared in the > expression being examined. > > After removing the dummy zero above, this kind of cycle in the > equivalence graph is no longer introduced by VTA itself, but dummy > zeros are also present in generated code, such as in the 32-bit x86's > pro_epilogue_adjust_stack_si_add epilogue insn generated as part of > the builtin longjmp in _Unwind_RaiseException building libgcc's > unwind-dw2.o. So, break the recursion cycle for them too. > > > for gcc/ChangeLog > > PR debug/67355 > * var-tracking.c (reverse_op): Don't add dummy zero to reverse > ops that simplify back to the original value. > * alias.c (refs_newer_value_p): Cut off recursion for > expressions containing the original value. OK. Presumably there's no good way to directly test this, even though we have a good sense of what's happening. Thus no testcase. Insert plug about unit testing here. Jeff