From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119598 invoked by alias); 24 Jul 2017 18:31:24 -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 118964 invoked by uid 89); 24 Jul 2017 18:31:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=adhere X-HELO: mail-wm0-f48.google.com Received: from mail-wm0-f48.google.com (HELO mail-wm0-f48.google.com) (74.125.82.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Jul 2017 18:31:21 +0000 Received: by mail-wm0-f48.google.com with SMTP id c184so12451733wmd.0 for ; Mon, 24 Jul 2017 11:31:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:user-agent:in-reply-to:references :mime-version:content-transfer-encoding:subject:to:from:message-id; bh=EOyHhpNIRge7LArI5DQ8BgkTQ7NRb+t9vVNL7V3BUds=; b=QEbFdNwe5Ym0aFsg5tcJcDSW/VYeO99XdNeLUiXgm6IQJMrfVR5duB8BaiGq3D2vwN m2zRKvkxGHUF2AO6cDQc75AOoM0vpzaZ7dTXuBmLUCywhysUznUf6KmGO/sooRD3Zxzp GnwuUkzDOqSqlZXbp43QRLVliTbYx1Y1/SZnm8V/TNms9+9cFVvxz2s4GMIE9NP0Huyz ZW8yj9AxaI79X06VsPd9mPJkQqTTxuhl7mCzNfhJ9oKSgOUCq0N6mSA67nUANRQi3XQ+ v+ZUe3p6Hp0CsVfkJeogsy1HsX34CUqJHy7ADd/7WO5PgHEeSoy86TzFctoHNV4ZMxF1 j1XQ== X-Gm-Message-State: AIVw112ZIxRiQHQTvkywTgYAh5dvozkSNPYyOX0yy2v7AyOXmGMMTZbk PvvUoQb5qHkdjEJ9nZA= X-Received: by 10.28.131.3 with SMTP id f3mr5810223wmd.181.1500921078844; Mon, 24 Jul 2017 11:31:18 -0700 (PDT) Received: from android-4c5a376a18c0e957.fritz.box (p5494E583.dip0.t-ipconnect.de. [84.148.229.131]) by smtp.gmail.com with ESMTPSA id v8sm13680881wrv.16.2017.07.24.11.31.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Jul 2017 11:31:17 -0700 (PDT) Date: Mon, 24 Jul 2017 18:31:00 -0000 User-Agent: K-9 Mail for Android In-Reply-To: <20170724134230.GY13471@gate.crashing.org> References: <8760ewohsv.fsf@linaro.org> <20170722210245.GP13471@gate.crashing.org> <87r2x6rxp5.fsf@linaro.org> <20170724105620.GX13471@gate.crashing.org> <87lgnero7y.fsf@linaro.org> <20170724134230.GY13471@gate.crashing.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [00/77] Add wrapper classes for machine_modes To: gcc-patches@gcc.gnu.org,Segher Boessenkool ,richard.sandiford@linaro.org From: Richard Biener Message-ID: <431B3B88-E71B-42E0-A8B8-87B180231734@gmail.com> X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg01444.txt.bz2 On July 24, 2017 3:42:30 PM GMT+02:00, Segher Boessenkool wrote: >On Mon, Jul 24, 2017 at 01:52:49PM +0100, Richard Sandiford wrote: >> Segher Boessenkool writes: >> > On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote: >> >> > From what I can tell so far it makes things much harder to read. >> >> > Perhaps that is just because this is all new. >> >>=20 >> >> Which parts specifically? E.g. is it mostly the is_a (x, &y) >changes? >> >> Or the as_a (x) changes too? Do you think the FOR_EACH_* >iterators >> >> also make things harder to read? Or is >machine_mode->scalar_int_mode >> >> itself a problem? >> > >> > All the as_a (x) etc. looks like cuneiform to me (not just in >your >> > patch); and I cannot read cuneiform :-) >> > >> > One day I might understand why we need all this C++ inverted >syntax, >> > needless abstraction, needless generalisation, data hiding and >everything >> > else hiding. Until then, I rant. Sorry. >> > >> > The main purpose of abstraction is to make code easier to >understand and >> > to write and change, but with C++ it usually makes it harder it >seems :-( >>=20 >> Well, as someone who was/is on the fence about the C++ thing, I >definitely >> sympathise :-) But in this particular case it really isn't (supposed >to be) >> "hey, we're using C++ now, let's add more layers!" abstraction. > >:-) > >> The same principle would have worked in C and IMO been useful in C. >> It sounds like you don't necessarily object to SCALAR_INT_TYPE_MODE >etc. >> as asserting forms of TYPE_MODE, so if the syntax is a problem, I >think: >>=20 >> as_a (x) =3D> force_scalar_int (x) >> is_a (x, &y) =3D> is_scalar_int (x, &y) >>=20 >> would be fine too, and shorter to write. Would that be better? > >Yeah that looks better, to me at least. I don't like that. We've settled on one style so please adhere to that. F= orce_ also suggests some magic semantics that are not there. So if any then try to improve the as-is stuff in general. For example it's= quite awkward in switches. Richard. >> Like I say, one advantage of the new wrappers is that they force mode >> assumptions to be explicit (otherwise the compiler won't build).=20 >That >> caught several real bugs before they had been found and fixed >upstream. >> But that argument might be too weak to support this on its own. > >It also helps compile time you said. I like that, too :-) > >> The other -- original, and IMO more compelling -- motivation is to >make >> it easier to add variable-sized modes without having to worry about >the >> possibility that scalar or complex modes could be variable-sized >> (because that would be much more invasive). We could instead have >kept >> everything as machine_mode, made GET_MODE_SIZE always be variable, >and >> forced the result to a constant whenever it's "known" to be constant >> for some reason. The difficulty with that is that it would be very >hard >> to get right if not testing SVE, and even with SVE you would need >just >> the right input code to trigger the problem. So what we wanted to do >> was make it "easy" for people to know when variable-sized modes were >a >> concern even if they weren't thinking about or testing SVE. This >> scalar/not scalar or complex/not complex choices aren't architecture- >> specific in the same way. >>=20 >> With this approach we only needed to force a variable size or offset >to >> a constant in a few places, and those cases were easy to justify from >> context. > >Yes, it is clear some changes are needed -- I just hope the changes >can make the code easier to understand, instead of more complex. > > >Segher