From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122257 invoked by alias); 25 Jul 2017 08:47:47 -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 122086 invoked by uid 89); 25 Jul 2017 08:47:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf0-f45.google.com Received: from mail-lf0-f45.google.com (HELO mail-lf0-f45.google.com) (209.85.215.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Jul 2017 08:47:44 +0000 Received: by mail-lf0-f45.google.com with SMTP id m86so46511628lfi.4 for ; Tue, 25 Jul 2017 01:47:44 -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=oHvV/67QintLbrt48sHfAaEuP3ITNILemasbxfj1ZvI=; b=IEWq4LdmZRKOlSh9JH64HhPSwAhKQ/BR0JhimWVtRyQlu1aXscBdAPhjrKaZhaMeda D1cyJM4eO5yZ8YvixrUGc0yDxhDpbHumEmNlqJ+fbuCd6Iqmc/4gYIJ4QFg8uccz7c1F 1zIJvbHpXIjYKPsXqStC7elwpZbzibk3mtFrNaJSZj/8kndx0V0NFjJ/SAQPdxSLMBY1 WB9JcP84QG18vXzO6qEOwpIpcbcl2GdI99OHYCEmmz2RE6cerThA9snV655VlnpVAo7M WNGMpkFi8jsJjFRA8dfaFRNsk4c328AitmzXBSaQMirc2Z9/2Ow6lKz9RsmxyaFicY31 Xetg== X-Gm-Message-State: AIVw113LC7K7ff6RtRztMtyanGkux9L3AVwZSyXbIhCBdZbL1XudIAt8 jGS/j0xLRTI/0horxF+5Ci/bZ8KSFw== X-Received: by 10.46.78.18 with SMTP id c18mr3121445ljb.126.1500972462111; Tue, 25 Jul 2017 01:47:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.31.134 with HTTP; Tue, 25 Jul 2017 01:47:41 -0700 (PDT) In-Reply-To: <59770299.8060501@foss.arm.com> References: <20170715204749.24398-1-amonakov@ispras.ru> <20170715204749.24398-3-amonakov@ispras.ru> <20170722111512.GM13471@gate.crashing.org> <59770299.8060501@foss.arm.com> From: Richard Biener Date: Tue, 25 Jul 2017 08:47:00 -0000 Message-ID: Subject: Re: [PATCH 2/6] gimple-ssa-store-merging.c: fix sort_by_bitpos To: Kyrill Tkachov Cc: Alexander Monakov , Segher Boessenkool , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg01506.txt.bz2 On Tue, Jul 25, 2017 at 10:34 AM, Kyrill Tkachov wrote: > Hi, > > On 24/07/17 21:48, Alexander Monakov wrote: >> >> On Sat, 22 Jul 2017, Segher Boessenkool wrote: >> >>> On Sat, Jul 15, 2017 at 11:47:45PM +0300, Alexander Monakov wrote: >>>> >>>> --- a/gcc/gimple-ssa-store-merging.c >>>> +++ b/gcc/gimple-ssa-store-merging.c >>>> @@ -516,12 +516,12 @@ sort_by_bitpos (const void *x, const void *y) >>>> store_immediate_info *const *tmp = (store_immediate_info * const *) >>>> x; >>>> store_immediate_info *const *tmp2 = (store_immediate_info * const *) >>>> y; >>>> - if ((*tmp)->bitpos <= (*tmp2)->bitpos) >>>> + if ((*tmp)->bitpos < (*tmp2)->bitpos) >>>> return -1; >>>> else if ((*tmp)->bitpos > (*tmp2)->bitpos) >>>> return 1; >>>> - >>>> - gcc_unreachable (); >>>> + else >>>> + return 0; >>>> } >>> >>> Does any sort using this comparison function require the sort to be >>> stable? >>> >>> It looks like the <= was simply a typo and should have been <, but >>> the gcc_unreachable was as intended? (With <= it is trivially >>> unreachable there). >> >> I think it's best if the original author can chime in - adding Kyrill. >> >> (to be clear, equal bitpos is possible, this issue causes qsort checker >> errors) > > > For the uses of this function the order when the bitpos is the same > does not matter, I just wanted to avoid returning zero to avoid > perturbations > due to qsort. > IMO the right thing to do here to avoid the qsort checker errors is to break > the tie between > store_immediate_info objects with equal bitpos by using the sort_by_order > heuristic > i.e. something like "return (*tmp)->order - (*tmp2)->order;". > That should give well-behaved results as the order of two > store_immediate_info objects > is guaranteed not to be the same (otherwise something has gone wrong > elsewhere). Agreed. Richard. > Thanks, > Kyrill > >> Alexander > >