On Wed, 11 May 2011, Julian Brown wrote:
> non-saturating variants where appropriate). Other functionality is
> provided by support functions in libgcc (type conversions,
> comparisons, and so on). Although more efficient versions of some of
> these could be implemented inline, it seems to me they would mostly use
> regular integer operations: I'm not at all sure that building that into
> each target rather than teaching generic code how to do it is a
> sensible way to go.
Indeed, the original fixed-point code seems rather badly thought out in
that way. Ideally you wouldn't have special machine modes at all; you'd
have extra operations on ordinary modes, and some GIMPLE pass, or expand
at the latest, would lower operations on fixed-point types into operations
on ordinary types. (You'd want appropriate representations in GIMPLE for
e.g. saturating operations, see PR 48580; in addition to basic saturating
operations, multiplication on fixed-point types corresponds to
multiply/shift on generic types.)
I think that would do a better job of making instruction set features
available to C code - ordinary C code using existing C idioms for
saturation or overflow checks in some cases, rather than just code using
these special extensions - where appropriate descriptions have been added
to the target. But it's certainly a lot more work.
> There's currently no support for "long long" fixed-point types, since
> those imply support for TImode arithmetic, which isn't sensible on ARM.
> I've modified tests to make optional the requirement for long long
> support.
I wonder how those tests work, or don't work, on 32-bit MIPS, which ought
to have the same issue....
> * optabs.c (prepare_cmp_insn): Use correct biasing for fixed-point
> comparison helpers.
> * final.c (output_addr_const): Print fixed-point constants as
> decimal not hex (avoiding an assembler overflow warning for
> negative byte constants).
> * tree-ssa-sccvn.c (copy_reference_ops_from_ref): Support FIXED_CST.
> * config/fixed-bit.h (fixed-bit-machine.h): Include.
> (DECORATE_FIXED_NAME): Conditionally define if undefined. Use
> throughout file to decorate function names.
> * config/fixed-bit-mach-generic.h: New.
It might make sense to send the machine-independent parts separately,
under a separate subject heading, with an explanation for why each bit is
needed.
> libgcc/
> * config.host (arm*-*-linux*, arm*-*-uclinux*, arm*-*-eabi*)
> (arm*-*-nucleuseabi*, arm*-*-symbianelf*): Add arm/t-fixed-point
The arm*-*-nucleuseabi* reference seems bogus.
> +/* Return TRUE if fixed-point operations are supported. */
> +
> +bool
> +arm_fixed_point_supported_p (void)
> +{
> + /* At least some operations helpful for fixed-point support are available
> + from arch3m onwards -- 32 bit x 32 bit -> 64 bit multiplies. There's
> not
> + much point in arbitrarily restricting fixed-point support to particular
> + architecture versions though since gaps can be filled by libcalls, so
> + return TRUE unconditionally. */
> + return true;
> +}
Is there a reason for having a separate function, rather than just using
hook_bool_void_true (with this comment going on the definition of
TARGET_FIXED_POINT_SUPPORTED_P)?
I don't see any change to the code in gcc/configure.ac that handles
--enable-fixed-point to know that ARM targets now support it. Since the
default hook returns ENABLE_FIXED_POINT, maybe if you fixed the configure
test you wouldn't need to define the hook for ARM at all....
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 40ebf35..8a1dd76 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -10751,3 +10751,5 @@
> (include "neon.md")
> ;; Synchronization Primitives
> (include "sync.md")
> +;; Fixed-point patterns
> +(include "arm-fixed.md")
Should MD_INCLUDES in t-arm be updated?
> diff --git a/gcc/config/arm/fixed-bit-machine.h
> b/gcc/config/arm/fixed-bit-machine.h
> new file mode 100644
> index 0000000..c7ce8ec
> --- /dev/null
> +++ b/gcc/config/arm/fixed-bit-machine.h
> @@ -0,0 +1,29 @@
> +/* Machine-specific override for fixed-point support routine names.
> + Copyright (C) 2011 Free Software Foundation, Inc.
> +
> + This file is part of GCC.
> +
> + GCC is free software; you can redistribute it and/or modify it
> + under the terms of the GNU General Public License as published
> + by the Free Software Foundation; either version 3, or (at your
> + option) any later version.
> +
> + GCC is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
> + License for more details.
> +
> + Under Section 7 of GPL version 3, you are granted additional
> + permissions described in the GCC Runtime Library Exception, version
> + 3.1, as published by the Free Software Foundation.
> +
> + You should have received a copy of the GNU General Public License
> + along with GCC; see the file COPYING3. If not see
> + <http://www.gnu.org/licenses/>. */
Wrong version of the last paragraph. See e.g. libgcc2.c. "GNU General
Public License and a copy of the GCC Runtime Library Exception ...
COPYING3 and COPYING.RUNTIME respectively".
> diff --git a/gcc/config/fixed-bit-mach-generic.h
> b/gcc/config/fixed-bit-mach-generic.h
> new file mode 100644
> index 0000000..c25f0a5
> --- /dev/null
> +++ b/gcc/config/fixed-bit-mach-generic.h
> @@ -0,0 +1,22 @@
> +/* Null machine-specific override for fixed-point support routine names.
> + Copyright (C) 2011 Free Software Foundation, Inc.
> +
> + This file is part of GCC.
> +
> + GCC is free software; you can redistribute it and/or modify it
> + under the terms of the GNU General Public License as published
> + by the Free Software Foundation; either version 3, or (at your
> + option) any later version.
> +
> + GCC is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
> + License for more details.
> +
> + Under Section 7 of GPL version 3, you are granted additional
> + permissions described in the GCC Runtime Library Exception, version
> + 3.1, as published by the Free Software Foundation.
> +
> + You should have received a copy of the GNU General Public License
> + along with GCC; see the file COPYING3. If not see
> + <http://www.gnu.org/licenses/>. */
Likewise.
--
Joseph S. Myers
jos...@codesourcery.com
