From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125678 invoked by alias); 4 Jul 2017 11:50:35 -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 125659 invoked by uid 89); 4 Jul 2017 11:50:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-16.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:2356 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 ESMTP; Tue, 04 Jul 2017 11:50:29 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1E47E80474; Tue, 4 Jul 2017 11:50:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1E47E80474 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1E47E80474 Received: from tucnak.zalov.cz (ovpn-116-143.ams2.redhat.com [10.36.116.143]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BC0175C7C1; Tue, 4 Jul 2017 11:50:27 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v64BoPtO006239; Tue, 4 Jul 2017 13:50:25 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v64BoNlL006238; Tue, 4 Jul 2017 13:50:23 +0200 Date: Tue, 04 Jul 2017 11:50:00 -0000 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix -fcompare-debug issues caused by recent VRP assert expr sorting changes (PR debug/81278) Message-ID: <20170704115023.GL2123@tucnak> Reply-To: Jakub Jelinek References: <20170704113247.GI2123@tucnak> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00192.txt.bz2 On Tue, Jul 04, 2017 at 01:46:25PM +0200, Richard Biener wrote: > > 2017-07-04 Jakub Jelinek > > > > PR debug/81278 > > * tree-vrp.c (compare_assert_loc): Only test if a->e is NULL, > > !a->e == !b->e has been verified already. Use e == NULL or > > e != NULL instead of e or ! e tests. > > (compare_assert_loc_stable): New function. > > (process_assert_insertions): Sort using compare_assert_loc_stable > > before calling process_assert_insertions_for in a loop. Use break > > instead of continue once seen NULL pointer. > > > > --- gcc/tree-vrp.c.jj 2017-07-03 19:03:23.817824263 +0200 > > +++ gcc/tree-vrp.c 2017-07-04 10:30:36.403204757 +0200 > > @@ -6400,13 +6400,13 @@ compare_assert_loc (const void *pa, cons > > { > > assert_locus * const a = *(assert_locus * const *)pa; > > assert_locus * const b = *(assert_locus * const *)pb; > > - if (! a->e && b->e) > > + if (a->e == NULL && b->e != NULL) > > return 1; > > - else if (a->e && ! b->e) > > + else if (a->e != NULL && b->e == NULL) > > return -1; > > > > /* Sort after destination index. */ > > - if (! a->e && ! b->e) > > + if (a->e == NULL) > > ; > > else if (a->e->dest->index > b->e->dest->index) > > return 1; > > so this will now ICE if b->e is NULL, did you forget the && b->e == NULL > above? That was intentional. If a->e != NULL, then we know that b->e != NULL, because we have else if (a->e != NULL && b->e == NULL) return -1; earlier. Similarly, if a->e == NULL, then we know that b-> == NULL, because we have: if (a->e == NULL && b->e != NULL) return 1; earlier. > > @@ -6506,11 +6547,12 @@ process_assert_insertions (void) > > } > > } > > > > + asserts.qsort (compare_assert_loc_stable); > > please add a comment here why we re-sort. Ok, will do. > > for (unsigned j = 0; j < asserts.length (); ++j) > > { > > loc = asserts[j]; > > if (! loc) > > - continue; > > + break; > > update_edges_p |= process_assert_insertions_for (ssa_name (i), loc); > > num_asserts++; > > free (loc); > > Otherwise looks ok to me. I wonder if we should merge the two > sorting functions and change behavior with a global var or a > template parameter instead (to reduce source duplication). Does > > vec.qsort (function_template); > > work? Let me try that. Jakub