From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40149 invoked by alias); 24 May 2017 14:28:03 -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 40130 invoked by uid 89); 24 May 2017 14:28:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=continually, vein X-HELO: mail-wm0-f44.google.com Received: from mail-wm0-f44.google.com (HELO mail-wm0-f44.google.com) (74.125.82.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 May 2017 14:28:00 +0000 Received: by mail-wm0-f44.google.com with SMTP id e127so76839384wmg.1 for ; Wed, 24 May 2017 07:28:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=eSDKng9TrgPZRQ5Dy6W8XtHfnw/hwZ6XlCot9xNlDP0=; b=Vk4dw1dclLgpRCaF8D48MOvNWeRnQmTxV1LQ+FVrA4WJg2lBh4VxsuEQC3oUMO0u66 SAb3VWSydSJIsOqI8d2NZsoMqx2k7ssmzB8+bD6CWEgM4WnHBaIsqzi24VHA+Nb3YMqk aorGVQEmC9kXFDmpjaLAYKdgqnHyWZwn2SJdi6uUNngCUMN7udKQ62OUh3GLKQMOzCSG DifdcCVThJOK+SIMYXuIewA8VXdwxr9Kmwx714WWXLhEGAfAGOiG0iV8XobbgZ5TqnEf NMOT/slZME+Kaz0MN9MkT5Phepas6/qZGgnU3j3ya1ky5z58gz/k9EP9tpfqVaFyDsQU qACw== X-Gm-Message-State: AODbwcDTmq3Ctzvd2pVjYV8PmI/QaU3gMz8jnGRj0oqR+G1dS6Y/jCGd 0JeEVAfG6xM2iBZmSELcjw== X-Received: by 10.28.157.11 with SMTP id g11mr6068483wme.113.1495636081750; Wed, 24 May 2017 07:28:01 -0700 (PDT) Received: from localhost (188.29.165.148.threembb.co.uk. [188.29.165.148]) by smtp.gmail.com with ESMTPSA id h199sm4415795wme.4.2017.05.24.07.28.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 May 2017 07:28:00 -0700 (PDT) From: Richard Sandiford To: Jeff Law Mail-Followup-To: Jeff Law ,gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org Subject: Re: [0/67] Add wrapper classes for machine_modes References: <87h96dp8u6.fsf@e105548-lin.cambridge.arm.com> <59d78c7c-0a59-ed02-4d90-8a749b328091@redhat.com> Date: Wed, 24 May 2017 14:33:00 -0000 In-Reply-To: <59d78c7c-0a59-ed02-4d90-8a749b328091@redhat.com> (Jeff Law's message of "Thu, 4 May 2017 22:20:51 -0600") Message-ID: <8760gqz5w3.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-05/txt/msg01878.txt.bz2 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. 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. Thanks, Richard