Re: [ARM] Refactor Neon Builtins infrastructure

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

On Wed, Nov 12, 2014 at 5:08 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> Hi,
>
> I was taking a look at fixing the issues in the ARM back-end exposed
> by Marc Glisse's patch in [1], and hoped to fix them by adapting the
> patch recently commited by Tejas ([2]).
>
> As I looked, I realised that the ARM target and the AArch64 target
> now differ drastically in how their Advanced SIMD builtin
> initialisation and expansion logic works. This is a growing
> maintenance burden. This patch series is an attempt to start fixing
> the problem.
>
> From a high level, I see five problems with the ARM Neon builtin code.
>
> First is the "magic number" interface, which gives builtins with signed
> and unsigned, or saturating and non-saturating, variants an extra
> parameter used to control which instruction is ultimately emitted. This
> is problematic as it enforces that these intrinsics be implemented with
> an UNSPEC pattern, we would like the flexibility to try to do a better job
> of modeling these patterns.
>
> Second, is that all the code lives in arm.c. This file is huge and
> frightening. The least we could do is start to split it up!
>
> Third, is the complicated builtin initialisation code. If we collect
> common cases together from the large switch in the initialisation function,
> it is clear we can eliminate much of the existing code. In fact, we have
> already solved the same problem in AArch64 ([3]), and we don't gain
> anything from having these interfaces separate.
>
> Fourth, is that we don't have infrastructure to strongly type the functions
> in arm_neon.h -  instead casting around between signed and unsigned vector
> arguments as required. We need this to avoid special casing some builtins
> we may want to vectorize (bswap and friends). Again we've solved this
> in AArch64 ([4]).
>
> Finally, there are the issues with type mangling Marc has seen.
>
> This patch-set tries to fix those issues in order, and progresses as so:
>
> First the magic words:
>
>   [Refactor Builtins: 1/8] Remove arm_neon.h's "Magic Words"
>
> Then moving code out to arm-builtins.c:
>
>   [Patch ARM Refactor Builtins 2/8] Move Processor flags to arm-protos.h
>   [Patch ARM Refactor Builtins 3/8] Pull builtins code to its own file
>
> And then making the ARM backend look like the AArch64 backend and fixing
> Marc's issue.
>
>   [Patch ARM Refactor Builtins 4/8]  Refactor "VAR<n>" Macros
>   [Patch ARM Refactor Builtins 5/8] Start keeping track of qualifiers in
>     ARM.
>   [Patch ARM Refactor Builtins 6/8] Add some tests for "poly" mangling
>   [Patch ARM Refactor Builtins 7/8] Use qualifiers arrays when
>     initialising builtins and fix type mangling
>   [Patch ARM Refactor Builtins 8/8] Neaten up the ARM Neon builtin
>     infrastructure
>
> Clearly there is more we could do to start sharing code between the two
> targets rather than duplicating it. For now, the benefit did not seem worth
> the substantial churn that this would cause both back-ends.
>
> I've bootstrapped each patch in this series in turn for both arm and
> thumb on arm-none-linux-gnueabihf.
>
> OK for trunk?

This is OK thanks.



regards
Ramana

>
> Thanks,
> James
>
> ---
> [1]: [c++] typeinfo for target types
>      https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00618.html
> [2]: [AArch64, Patch] Restructure arm_neon.h vector types's implementation
>      https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00264.html
> [3]: [AArch64] Refactor Advanced SIMD builtin initialisation.
>      https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00532.html
> [4]: [AArch64] AArch64 SIMD Builtins Better Type Correctness.
>      https://gcc.gnu.org/ml/gcc-patches/2013-11/msg02005.html
>
> ---
>  gcc/config.gcc                               |    3 +-
>  gcc/config/arm/arm-builtins.c                | 2925 ++++++++++++++++++++++++
>  gcc/config/arm/arm-protos.h                  |  173 +-
>  gcc/config/arm/arm-simd-builtin-types.def    |   48 +
>  gcc/config/arm/arm.c                         | 3149 +-------------------------
>  gcc/config/arm/arm_neon.h                    | 1743 +++++++-------
>  gcc/config/arm/arm_neon_builtins.def         |  435 ++--
>  gcc/config/arm/iterators.md                  |  167 ++
>  gcc/config/arm/neon.md                       |  893 ++++----
>  gcc/config/arm/t-arm                         |   11 +
>  gcc/config/arm/unspecs.md                    |  109 +-
>  gcc/testsuite/g++.dg/abi/mangle-arm-crypto.C |   16 +
>  gcc/testsuite/g++.dg/abi/mangle-neon.C       |    5 +
>  gcc/testsuite/gcc.target/arm/pr51968.c       |    2 +-
>  create mode 100644 gcc/config/arm/arm-builtins.c
>  create mode 100644 gcc/config/arm/arm-simd-builtin-types.def
>  create mode 100644 gcc/testsuite/g++.dg/abi/mangle-arm-crypto.C
>  14 files changed, 4992 insertions(+), 4687 deletions(-)