From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35180 invoked by alias); 9 Dec 2016 13:00:00 -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 35153 invoked by uid 89); 9 Dec 2016 12:59:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wj0-f194.google.com Received: from mail-wj0-f194.google.com (HELO mail-wj0-f194.google.com) (209.85.210.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 12:59:57 +0000 Received: by mail-wj0-f194.google.com with SMTP id j10so2404947wjb.3 for ; Fri, 09 Dec 2016 04:59:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=DA2T55PpYVRbDLJqxYjpmpqsMHskB9x8dJRychZvhTE=; b=gneW6zkJkxf1UYC2WvOwbOi+iNlcxptkqeye2ClF3Oysdg4eYAjQFyExnMN9ogS8pw rC7XrulIVlIOgLdazrINMp8RXZw3IyLCFFX/1XpUa3RSnozR2V1q+8tNidQ8Rdtpd0fA oGgt2GnN0HfMlzO58+UbhIJF391AdyR6il0mMJTHA6i2R6k9QnbxZghIBCZHMCLieYbF 1+jH45y7doR18Qz83W7yYzl6bIqWRo/jyoQVKpB8yZ0ACNYFCdialuDPJgMKpJu1TD/k r5P+BjrvxAW1W8tLRJirsXcxAzDYYFEqjza4jY8sAEiblSJ0sF3eeOPP9Kl1j4VCLt3I JnfA== X-Gm-Message-State: AKaTC01d29vkD1+PMElfG1gQvMazlEykgRqKHO1SCh1GnyuF5bNUzvkxWAO3FJMN/W1NYxU9U/VUNNMoxb/0Mg== X-Received: by 10.195.11.41 with SMTP id ef9mr21183095wjd.89.1481288394750; Fri, 09 Dec 2016 04:59:54 -0800 (PST) MIME-Version: 1.0 Received: by 10.28.137.2 with HTTP; Fri, 9 Dec 2016 04:59:54 -0800 (PST) In-Reply-To: <87h96dp8u6.fsf@e105548-lin.cambridge.arm.com> References: <87h96dp8u6.fsf@e105548-lin.cambridge.arm.com> From: Richard Biener Date: Fri, 09 Dec 2016 13:00:00 -0000 Message-ID: Subject: Re: [0/67] Add wrapper classes for machine_modes To: GCC Patches , richard.sandiford@arm.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00780.txt.bz2 On Fri, Dec 9, 2016 at 1:48 PM, 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 > > Sorry that this is so late, been distracted by other things. Even if > we're too far into stage 3 for SVE itself to go in, I was hoping this > part (which was kind-of posted during stage 1) could go in independently. What's the benefit of taking this but not SVE? I'd say not at this stage please. Richard. > Thanks, > Richard