From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69631 invoked by alias); 6 Dec 2017 20:11:55 -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 69613 invoked by uid 89); 6 Dec 2017 20:11:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=sticking, arguably, negligible, reputation 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; Wed, 06 Dec 2017 20:11:53 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1F60CC0587C1; Wed, 6 Dec 2017 20:11:52 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-2.rdu2.redhat.com [10.10.112.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3908762660; Wed, 6 Dec 2017 20:11:50 +0000 (UTC) Subject: Re: [001/nnn] poly_int: add poly-int.h To: gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org References: <871sltvm7r.fsf@linaro.org> <87vaj5u7id.fsf@linaro.org> <87efp95c9b.fsf@linaro.org> <87375hvi77.fsf@linaro.org> From: Jeff Law Message-ID: Date: Wed, 06 Dec 2017 20:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <87375hvi77.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00344.txt.bz2 On 11/13/2017 05:04 PM, Richard Sandiford wrote: > Richard Sandiford writes: >> Richard Sandiford writes: >>> This patch adds a new "poly_int" class to represent polynomial integers >>> of the form: >>> >>> C0 + C1*X1 + C2*X2 ... + Cn*Xn >>> >>> It also adds poly_int-based typedefs for offsets and sizes of various >>> precisions. In these typedefs, the Ci coefficients are compile-time >>> constants and the Xi indeterminates are run-time invariants. The number >>> of coefficients is controlled by the target and is initially 1 for all >>> ports. >>> >>> Most routines can handle general coefficient counts, but for now a few >>> are specific to one or two coefficients. Support for other coefficient >>> counts can be added when needed. >>> >>> The patch also adds a new macro, IN_TARGET_CODE, that can be >>> set to indicate that a TU contains target-specific rather than >>> target-independent code. When this macro is set and the number of >>> coefficients is 1, the poly-int.h classes define a conversion operator >>> to a constant. This allows most existing target code to work without >>> modification. The main exceptions are: >>> >>> - values passed through ..., which need an explicit conversion to a >>> constant >>> >>> - ?: expression in which one arm ends up being a polynomial and the >>> other remains a constant. In these cases it would be valid to convert >>> the constant to a polynomial and the polynomial to a constant, so a >>> cast is needed to break the ambiguity. >>> >>> The patch also adds a new target hook to return the estimated >>> value of a polynomial for costing purposes. >>> >>> The patch also adds operator<< on wide_ints (it was already defined >>> for offset_int and widest_int). I think this was originally excluded >>> because >> is ambiguous for wide_int, but << is useful for converting >>> bytes to bits, etc., so is worth defining on its own. The patch also >>> adds operator% and operator/ for offset_int and widest_int, since those >>> types are always signed. These changes allow the poly_int interface to >>> be more predictable. >>> >>> I'd originally tried adding the tests as selftests, but that ended up >>> bloating cc1 by at least a third. It also took a while to build them >>> at -O2. The patch therefore uses plugin tests instead, where we can >>> force the tests to be built at -O0. They still run in negligible time >>> when built that way. >> >> Changes in v2: >> >> - Drop the controversial known_zero etc. wrapper functions. >> - Fix the operator<<= bug that Martin found. >> - Switch from "t" to "type" in SFINAE classes (requested by Martin). >> >> Not changed in v2: >> >> - Default constructors are still empty. I agree it makes sense to use >> "= default" when we switch to C++11, but it would be dangerous for >> that to make "poly_int64 x;" less defined than it is now. > > After talking about this a bit more internally, it was obvious that > the choice of "must" and "may" for the predicate names was a common > sticking point. The idea was to match the names of alias predicates, > but given my track record with names ("too_empty_p" being a recently > questioned example :-)), I'd be happy to rename them to something else. > Some alternatives we came up with were: I didn't find the must vs may naming problematical as I was going through the changes. What I did find much more difficult was determining if the behavior was correct when we used a "may" predicate. It really relies a good deal on knowing the surrounding code. In places where I knew the code reasonably well could tell without much surrounding context. In other places I had to look at the code and deduce proper behavior in the "may" cases -- and often I resorted to spot checking and relying on your reputation & testing to DTRT. > > - known_eq / maybe_eq / known_lt / maybe_lt etc. > > Some functions already use "known" and "maybe", so this would arguably > be more consistent than using "must" and "may". > > - always_eq / sometimes_eq / always_lt / sometimes_lt > > Similar to the previous one in intent. It's just a question of which > wordng is clearer. > > - forall_eq / exists_eq / forall_lt / exists_lt etc. > > Matches the usual logic quantifiers. This seems quite appealing, > as long as it's obvious that in: > > forall_eq (v0, v1) > > v0 and v1 themselves are already bound: if vi == ai + bi*X then > what we really saying is: > > forall X, a0 + b0*X == a1 + b1*X > > Which of those sounds best? Any other suggestions? I can live with any of them. I tend to prefer one of the first two, but it's not a major concern for me. So if you or others have a clear preference, go with it. jeff