This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add _Float<N>/_Float<N>X rounding built-ins & improve gimple optimization of _Float<N>/_Float<N>X built-in functions
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Michael Meissner <meissner at linux dot vnet dot ibm dot com>
- Cc: Segher Boessenkool <segher at kernel dot crashing dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Date: Thu, 21 Dec 2017 18:16:16 +0000
- Subject: Re: [PATCH] Add _Float<N>/_Float<N>X rounding built-ins & improve gimple optimization of _Float<N>/_Float<N>X built-in functions
- Authentication-results: sourceware.org; auth=none
- References: <20171117050445.GA13927@ibm-tiger.the-meissners.org> <20171117140608.GV10515@gate.crashing.org> <20171118003505.GA20542@ibm-tiger.the-meissners.org>
On Fri, 17 Nov 2017, Michael Meissner wrote:
> Here is the fixed patch. It fixes the btrunc<mode>2 insn to use the correct
> XSRPQI variant for truncf128. I added the float128-hw11.c test as a runtime
> test to make sure round, trunc, ceil, and floor return the correct values. The
> machine independent portions are the same.
The architecture-independent changes are OK. However, I have a comment on
the target parts:
> +(define_insn "round<mode>2"
> + [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
> + (unspec:IEEE128
> + [(match_operand:IEEE128 1 "altivec_register_operand" "v")]
> + UNSPEC_FRIN))]
> + "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
> + "xsrqpi 0,%0,%1,3"
> + [(set_attr "type" "vecfloat")
> + (set_attr "size" "128")])
My reading of Power ISA 3.0B documentation is that 0,%0,%1,3 means round
in the mode specified by FPSCR and you need 0,%0,%1,0 for
round-to-nearest-away semantics which are what the round<mode>2
instruction has (i.e., what you've written here is actually correct for
nearbyint<mode>2, and would be rint<mode>2 if xsrqpix were used instead).
Note that the testcase
> Index: gcc/testsuite/gcc.target/powerpc/float128-hw11.c
> +static const struct {
> + _Float128 value;
> + _Float128 exp_round;
> + _Float128 exp_floor;
> + _Float128 exp_ceil;
> + _Float128 exp_trunc;
> +} a[] = {
> + { -2.0Q, -2.0Q, -2.0Q, -2.0Q, -2.0Q },
> + { -1.7Q, -2.0Q, -2.0Q, -1.0Q, -1.0Q },
> + { -1.5Q, -2.0Q, -2.0Q, -1.0Q, -1.0Q },
> + { -1.3Q, -1.0Q, -2.0Q, -1.0Q, -1.0Q },
> + { +0.0Q, +0.0Q, +0.0Q, +0.0Q, +0.0Q },
> + { +1.3Q, +1.0Q, +1.0Q, +2.0Q, +1.0Q },
> + { +1.5Q, +2.0Q, +1.0Q, +2.0Q, +1.0Q },
> + { +1.7Q, +2.0Q, +1.0Q, +2.0Q, +1.0Q },
> + { +2.0Q, +2.0Q, +2.0Q, +2.0Q, +2.0Q }
> +};
has only -1.5Q and +1.5Q as half-way inputs, and both of those inputs have
the same results for round-to-nearest-even and round-to-nearest-away, and
the test doesn't test changing the rounding mode from the default
round-to-nearest-even. So getting this case wrong would not have shown up
with a test failure; you'd need to add a further test input such as 2.5
for which round-to-nearest-even and round-to-nearest-away differ, or test
in multiple rounding modes to verify that the results of these built-in
functions do not depend on the rounding mode, or both.
(Of course xsrqpi can also implement roundevenf128 using 1,%0,%1,0 but GCC
doesn't currently have any built-in function or machine description
support for roundeven for any type.)
--
Joseph S. Myers
joseph@codesourcery.com