This is the mail archive of the
mailing list for the GCC project.
Re: [0/67] Add wrapper classes for machine_modes
- From: Jeff Law <law at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org, richard dot sandiford at linaro dot org
- Date: Wed, 28 Jun 2017 11:26:51 -0600
- Subject: Re: [0/67] Add wrapper classes for machine_modes
- Authentication-results: sourceware.org; auth=none
- 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 at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 18B067EBD6
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 18B067EBD6
- References: <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org>
On 05/24/2017 08:27 AM, Richard Sandiford wrote:
> Jeff Law <email@example.com> writes:
>> On 12/09/2016 05:48 AM, Richard Sandiford wrote:
>>> This series includes most of the changes in group C from:
>>> 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
>>> 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:
>>> 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:
>> 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
> 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.