From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85083 invoked by alias); 28 Jun 2017 17:27:01 -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 85024 invoked by uid 89); 28 Jun 2017 17:26:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=recollection 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, 28 Jun 2017 17:26:56 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 18B067EBD6; Wed, 28 Jun 2017 17:26:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 18B067EBD6 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 18B067EBD6 Received: from localhost.localdomain (ovpn-117-103.phx2.redhat.com [10.3.117.103]) by smtp.corp.redhat.com (Postfix) with ESMTP id B009C955CD; Wed, 28 Jun 2017 17:26:52 +0000 (UTC) Subject: Re: [0/67] Add wrapper classes for machine_modes To: gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org References: <87h96dp8u6.fsf@e105548-lin.cambridge.arm.com> <59d78c7c-0a59-ed02-4d90-8a749b328091@redhat.com> <8760gqz5w3.fsf@linaro.org> From: Jeff Law Message-ID: <20ba1577-6bfe-cbdc-e917-35a7aa788df1@redhat.com> Date: Wed, 28 Jun 2017 17:27:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <8760gqz5w3.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg02194.txt.bz2 On 05/24/2017 08:27 AM, Richard Sandiford wrote: > Jeff Law writes: >> On 12/09/2016 05:48 AM, Richard Sandiford wrote: >>> This series includes most of the changes in group C from: >>> >>> https://gcc.gnu.org/ml/gcc/2016-11/msg00033.html >>> >>> The idea is to add wrapper classes around machine_mode_enum >>> for specific groups of modes, such as scalar integers, scalar floats, >>> complex values, etc. This has two main benefits: one specific to SVE >>> and one not. >>> >>> The SVE-specific benefit is that it helps to introduce the concept >>> of variable-length vectors. To do that we need to change the size >>> of a vector mode from being a known compile-time constant to being >>> (possibly) a run-time invariant. We then need to do the same for >>> unconstrained machine_modes, which might or might not be vectors. >>> Introducing these new constrained types means that we can continue >>> to treat them as having a constant size. >>> >>> The other benefit is that it uses static type checking to enforce >>> conditions that are easily forgotten otherwise. The most common >>> sources of problems seem to be: >>> >>> (a) using VOIDmode or BLKmode where a scalar integer was expected >>> (e.g. when getting the number of bits in the value). >>> >>> (b) simplifying vector operations in ways that only make sense for >>> scalars. >>> >>> The series helps with both of these, although we don't get the full >>> benefit of (b) until variable-sized modes are introduced. >>> >>> I know of three specific cases in which the static type checking >>> forced fixes for things that turned out to be real bugs (although >>> we didn't know that at the time, otherwise we'd have posted patches). >>> They were later fixed for trunk by: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01783.html >>> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02983.html >>> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02896.html >>> >>> The group C patches in ARM/sve-branch did slow compile time down a little. >>> I've since taken steps to avoid that: >>> >>> - Make the tailcall pass handle aggregate parameters and return values >>> (already in trunk). >>> >>> - Turn some of the new wrapper functions into inline functions. >>> >>> - Make all the machmode.h macros that used: >>> >>> __builtin_constant_p (M) ? foo_inline (M) : foo_array[M[ >>> >>> forward to an ALWAYS_INLINE function, so that (a) M is only evaluated >>> once and (b) __builtin_constant_p is applied to a variable, and so is >>> deferred until later passes. This helped the optimisation to fire in >>> more cases and to continue firing when M is a class rather than a >>> raw enum. >>> >>> - In a similar vein, make sure that conditions like: >>> >>> SImode == DImode >>> >>> are treated as builtin_constant_p by gencondmd, so that .md patterns >>> with those conditions are dropped. >>> >>> With these changes the series is actually a very slight compile-time win. >>> That might seem unlikely, but there are several possible reasons: >>> >>> 1. The machmode.h macro change above might allow more constant folding. >>> >>> 2. The series has a tendency to evaluate modes once, rather than >>> continually fetching them from (sometimes quite deep) rtx nests. >>> Refetching a mode is a particular problem if call comes between >>> two uses, since the compiler then has to re-evaluate the whole thing. >>> >>> 3. The series introduces many uses of new SCALAR_*TYPE_MODE macros, >>> as alternatives to TYPE_MODE. The new macros avoid the usual: >>> >>> (VECTOR_TYPE_P (TYPE_CHECK (NODE)) \ >>> ? vector_type_mode (NODE) : (NODE)->type_common.mode) >>> >>> and become direct field accesses in release builds. >>> >>> VECTOR_TYPE_P would be consistently false for these uses, >>> but call-clobbered registers would usually be treated as clobbered >>> by the condition as a whole. >>> >>> Maybe (3) is the most likely reason. >>> >>> I tested this by compiling the testsuite for: >>> >>> aarch64-linux-gnu alpha-linux-gnu arc-elf arm-linux-gnueabi >>> arm-linux-gnueabihf avr-elf bfin-elf c6x-elf cr16-elf cris-elf >>> epiphany-elf fr30-elf frv-linux-gnu ft32-elf h8300-elf >>> hppa64-hp-hpux11.23 ia64-linux-gnu i686-pc-linux-gnu >>> i686-apple-darwin iq2000-elf lm32-elf m32c-elf m32r-elf >>> m68k-linux-gnu mcore-elf microblaze-elf mips-linux-gnu >>> mipsisa64-linux-gnu mmix mn10300-elf moxie-rtems msp430-elf >>> nds32le-elf nios2-linux-gnu nvptx-none pdp11 powerpc-linux-gnuspe >>> powerpc-eabispe powerpc64-linux-gnu powerpc-ibm-aix7.0 rl78-elf >>> rx-elf s390-linux-gnu s390x-linux-gnu sh-linux-gnu sparc-linux-gnu >>> sparc64-linux-gnu sparc-wrs-vxworks spu-elf tilegx-elf tilepro-elf >>> xstormy16-elf v850-elf vax-netbsdelf visium-elf x86_64-darwin >>> x86_64-linux-gnu xtensa-elf >>> >>> and checking that there were no changes in assembly. Also tested >>> in the normal way on aarch64-linux-gnu and x86_64-linux-gnu. >>> >>> The series depends on the already-posted: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01657.html >> So can we get the discussion around the prerequisite restarted -- I like >> the core ideas around building wrapper classes around machine modes, but >> obviously we can't really move forward on this without the prereqs. > > Finally got back to this, sorry for the delay. I think the only > remaining prerequisite is the coretypes.h reorg patch. I'll post > an updated version of that in a sec. Oh the irony. I ping you, then have to disappear into the land of stack-clash :( > > I've also rebased and retested the patch series itself. Should I repost > it in one go, or split it up further? The original idea was to show that > this wasn't a half-transition and that the series on its own leaves things > in a sensible state. But it's also true that there are no forward > dependencies, so it'd also be possible to commit the patches in stages. I think in one go -- my recollection was that I already worked through much of it without seeing anything particularly worrisome. So in theory it shouldn't take much to swap that state back in. jeff