From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by sourceware.org (Postfix) with ESMTPS id 2DE0C3858D28 for ; Mon, 17 Jul 2023 13:48:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2DE0C3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=ispras.ru Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ispras.ru Received: from [10.10.3.121] (unknown [10.10.3.121]) by mail.ispras.ru (Postfix) with ESMTPS id 9BE5E407672C; Mon, 17 Jul 2023 13:48:30 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 9BE5E407672C Date: Mon, 17 Jul 2023 16:48:30 +0300 (MSK) From: Alexander Monakov To: Richard Biener cc: Tamar Christina , Andrew Pinski , Roger Sayle , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] Fix bootstrap failure (with g++ 4.8.5) in tree-if-conv.cc. In-Reply-To: Message-ID: References: <001201d9b684$e319fdd0$a94df970$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 17 Jul 2023, Richard Biener wrote: > > > > > OK. Btw, while I didn't spot this during review I would appreciate > > > > > if the code could use vec.[q]sort, this should work with a lambda as > > > > > well I think. > > > > > > > > That was my first use, but that hits > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99469 > > > > > > That is not hitting PR 99469 but rather it means your comparison is not > > > correct for an (unstable) sort. > > > That is qsort comparator should have this relationship `f(a,b) == !f(b, a)` and > > > `f(a,a)` should also return false. > > > > I'm using the standard std::pair comparator which indicates that f(a,a) is true, > > https://en.cppreference.com/w/cpp/utility/pair/operator_cmp > > > > > If you are running into this for qsort here, you will most likely run into issues > > > with std::sort later on too. > > > > Don't see why or how. It needs to have a consistent relationship which std::pair > > maintains. So why would using the standard tuple comparator with a standard > > std::sort cause problem? > > At least for > > return left.second < right.second; > > f(a,a) doesn't hold. Note qsort can end up comparing an element to > itself (not sure if GCCs implementation now can). (it cannot but that is not important here) Tamar, while std::sort receives a "less-than" comparison predicate, qsort needs a tri-state comparator that returns a negative value for "less-than" relation, positive for "more-than", and zero when operands are "equal". Passing output of std::pair::operator< straight to qsort is not correct, and qsort_chk catches that mistake at runtime. std::sort is not a stable sort and therefore can cause code generation differences by swapping around elements that are not bitwise-identical but "equal" according to the comparator. This is the main reason for preferring our internal qsort, which yields same results on all platforms. Let me also note that #include is pretty heavy-weight, and so I'd suggest to avoid it to avoid needlessly increasing bootstrap times. Alexander