This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add _Float<N>/_Float<N>X rounding built-ins & improve gimple optimization of _Float<N>/_Float<N>X built-in functions


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]