Re: Add option for whether ceil etc. can raise "inexact", adjust x86 conditions

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Aug 15, 2017 at 4:21 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Aug 15, 2017 at 3:52 PM, Martin Jambor <mjambor@suse.cz> wrote:
>> Hi Joseph,
>>
>> On Thu, May 26, 2016 at 09:02:02PM +0000, Joseph Myers wrote:
>>> On Thu, 26 May 2016, Jan Hubicka wrote:
>>>
>>> > > > +ffp-int-builtin-inexact
>>> > > > +Common Report Var(flag_fp_int_builtin_inexact) Optimization
>>> > > > +Allow built-in functions ceil, floor, round, trunc to raise \"inexact\" exceptions.
>>> >
>>> > When adding new codegen option which affects the correctness, it is also
>>> > necessary to update can_inline_edge_p and inline_call.
>>>
>>> This patch version adds handling for the new option in those places.
>>> Other changes: the default for the option is corrected so that
>>> -ffp-int-builtin-inexact really is in effect by default as intended;
>>> md.texi documentation for the patterns in question is updated to
>>> describe how they are affected by this option.
>>>
>>>
>>> Add option for whether ceil etc. can raise "inexact", adjust x86 conditions.
>>>
>>> In ISO C99/C11, the ceil, floor, round and trunc functions may or may
>>> not raise the "inexact" exception for noninteger arguments.  Under TS
>>> 18661-1:2014, the C bindings for IEEE 754-2008, these functions are
>>> prohibited from raising "inexact", in line with the general rule that
>>> "inexact" is only when the mathematical infinite precision result of a
>>> function differs from the result after rounding to the target type.
>>>
>>> GCC has no option to select TS 18661 requirements for not raising
>>> "inexact" when expanding built-in versions of these functions inline.
>>> Furthermore, even given such requirements, the conditions on the x86
>>> insn patterns for these functions are unnecessarily restrictive.  I'd
>>> like to make the out-of-line glibc versions follow the TS 18661
>>> requirements; in the cases where this slows them down (the cases using
>>> x87 floating point), that makes it more important for inline versions
>>> to be used when the user does not care about "inexact".
>>
>> Unfortunately, I have found out that this commit regresses run-time of
>> 538.imagick_r by about 5% on an AMD Ryzen machine and by 9% on a
>> slightly older Intel machine when compiled with just -O2 (so with
>> generic tuning).
>>
>> The problem is that ImageMagick spends a lot time calculating ceil and
>> floor and even with with generic tuning their library implementations
>> can use the ifunc mechanism to execute an efficient SSE 4.1
>> implementation on the processors that have it, whereas the inline
>> expansion cannot do so and is much bigger and much much slower.  To
>> give you an idea, this is the profile before and after the change:
>>
>>   | Symbol                           |  237073 | % of runtime |  237074 | % of runtime | sample delta | % sample delta |
>>   |----------------------------------+---------+--------------+---------+--------------+--------------+----------------|
>>   | MorphologyApply                  | 1058932 |       52.88% | 1043194 |       46.65% |       -15738 |          98.51 |
>>   | MeanShiftImage                   |  508088 |       25.50% |  833378 |       37.43% |       325290 |         164.02 |
>>   | GetVirtualPixelsFromNexus        |  173354 |        8.70% |  168298 |        7.56% |        -5056 |          97.08 |
>>   | SetPixelCacheNexusPixels.isra.10 |  114101 |        5.72% |  112790 |        5.07% |        -1311 |          98.85 |
>>   | __ceil_sse41                     |   21404 |        1.07% |       0 |            0 |       -21404 |           0.00 |
>>   | __floor_sse41                    |   19179 |        0.96% |       0 |            0 |       -19179 |           0.00 |
>>
>> And all of the sample count increases in MeanShiftImage can be tracked
>> down to the line in the cource calculating
>>
>>   if ((x-floor(x)) < (ceil(x)-x))
>>
>> I am not sure what to do about this, to me it seems that the
>> -ffp-int-builtin-inexact simply has a wrong default value, at least
>> for x86_64, as it was added in order not to slow code down but does
>> exactly that (all of the slowdown of course disappears when
>> -fno-fp-int-builtin-inexact is used).
>>
>> Or is the situation somehow more complex?
>
> I suppose these days the big inline sequences for the rounding functions
> are no longer profitable for generic tuning (assuming 'generic' nowadays
> includes SSE41 support).  Esp. floor/ceil includes jumpy compensation
> code.

Note other options are to inline if (__builtin_cpu_supports
("sse4.1")) ... else ...
or to emit a call to a (local? comdat?) __gcc_floor ifunc dispatcher and
emit the ifunc math library ourselves (like we'd do with attribute(target(""))).

Not sure if we really can assume glibc is intelligent enough -- does it have
non-SSE4.1 implementations for ceil/floor?  Back in time I implemented
these SSE2 expansions it used the generic C code which was awfully slow...

Richard.

> Note that (x - floor(x)) < (ceil(x) - x) looks like some clever simplification
> might speed it up.  Not that I can come up with sth off my head...
>
> Richard.
>
>> Martin
>>
>>
>>>
>>> This patch fixes these issues.  A new option
>>> -fno-fp-int-builtin-inexact is added to request TS 18661 rules for
>>> these functions; the default -ffp-int-builtin-inexact reflects that
>>> such exceptions are allowed by C99 and C11.  (The intention is that if
>>> C2x incorporates TS 18661-1, then the default would change in C2x
>>> mode.)
>>>
>>> The x86 built-ins for rint (x87, SSE2 and SSE4.1) are made
>>> unconditionally available (no longer depending on
>>> -funsafe-math-optimizations or -fno-trapping-math); "inexact" is
>>> correct for noninteger arguments to rint.  For floor, ceil and trunc,
>>> the x87 and SSE2 built-ins are OK if -ffp-int-builtin-inexact or
>>> -fno-trapping-math (they may raise "inexact" for noninteger
>>> arguments); the SSE4.1 built-ins are made to use ROUND_NO_EXC so that
>>> they do not raise "inexact" and so are OK unconditionally.
>>>
>>> Now, while there was no semantic reason for depending on
>>> -funsafe-math-optimizations, the insn patterns had such a dependence
>>> because of use of gen_truncxf<mode>2_i387_noop to truncate back to
>>> SFmode or DFmode after using frndint in XFmode.  In this case a no-op
>>> truncation is safe because rounding to integer always produces an
>>> exactly representable value (the same reason why IEEE semantics say it
>>> shouldn't produce "inexact") - but of course that insn pattern isn't
>>> safe because it would also match cases where the truncation is not in
>>> fact a no-op.  To allow frndint to be used for SFmode and DFmode
>>> without that unsafe pattern, the relevant frndint patterns are
>>> extended to SFmode and DFmode or new SFmode and DFmode patterns added,
>>> so that the frndint operation can be represented in RTL as an
>>> operation acting directly on SFmode or DFmode without the extension
>>> and the problematic truncation.
>>>
>>> A generic test of the new option is added, as well as x86-specific
>>> tests, both execution tests including the generic test with different
>>> x86 options and scan-assembler tests verifying that functions that
>>> should be inlined with different options are indeed inlined.
>>>
>>> I think other architectures are OK for TS 18661-1 semantics already.
>>> Considering those defining "ceil" patterns: aarch64, arm, rs6000, s390
>>> use instructions that do not raise "inexact"; nvptx does not support
>>> floating-point exceptions.  (This does mean the -f option in fact only
>>> affects one architecture, but I think it should still be a -f option;
>>> it's logically architecture-independent and is expected to be affected
>>> by future -std options, so is similar to e.g. -fexcess-precision=,
>>> which also does nothing on most architectures but is implied by -std
>>> options.)
>>>
>>> Bootstrapped with no regressions on x86_64-pc-linux-gnu.  OK to
>>> commit?
>>>
>>> gcc:
>>> 2016-05-26  Joseph Myers  <joseph@codesourcery.com>
>>>
>>>       PR target/71276
>>>       PR target/71277
>>>       * common.opt (ffp-int-builtin-inexact): New option.
>>>       * doc/invoke.texi (-fno-fp-int-builtin-inexact): Document.
>>>       * doc/md.texi (floor@var{m}2, btrunc@var{m}2, round@var{m}2)
>>>       (ceil@var{m}2): Document dependence on this option.
>>>       * ipa-inline-transform.c (inline_call): Handle
>>>       flag_fp_int_builtin_inexact.
>>>       * ipa-inline.c (can_inline_edge_p): Likewise.
>>>       * config/i386/i386.md (rintxf2): Do not test
>>>       flag_unsafe_math_optimizations.
>>>       (rint<mode>2_frndint): New define_insn.
>>>       (rint<mode>2): Do not test flag_unsafe_math_optimizations for 387
>>>       or !flag_trapping_math for SSE.  Just use gen_rint<mode>2_frndint
>>>       for 387 instead of extending and truncating.
>>>       (frndintxf2_<rounding>): Test flag_fp_int_builtin_inexact ||
>>>       !flag_trapping_math instead of flag_unsafe_math_optimizations.
>>>       Change to frndint<mode>2_<rounding>.
>>>       (frndintxf2_<rounding>_i387): Likewise.  Change to
>>>       frndint<mode>2_<rounding>_i387.
>>>       (<rounding_insn>xf2): Likewise.
>>>       (<rounding_insn><mode>2): Test flag_fp_int_builtin_inexact ||
>>>       !flag_trapping_math instead of flag_unsafe_math_optimizations for
>>>       x87.  Test TARGET_ROUND || !flag_trapping_math ||
>>>       flag_fp_int_builtin_inexact instead of !flag_trapping_math for
>>>       SSE.  Use ROUND_NO_EXC in constant operand of
>>>       gen_sse4_1_round<mode>2.  Just use gen_frndint<mode>2_<rounding>
>>>       for 387 instead of extending and truncating.
>>>
>>> gcc/testsuite:
>>> 2016-05-26  Joseph Myers  <joseph@codesourcery.com>
>>>
>>>       PR target/71276
>>>       PR target/71277
>>>       * gcc.dg/torture/builtin-fp-int-inexact.c,
>>>       gcc.target/i386/387-builtin-fp-int-inexact.c,
>>>       gcc.target/i386/387-rint-inline-1.c,
>>>       gcc.target/i386/387-rint-inline-2.c,
>>>       gcc.target/i386/sse2-builtin-fp-int-inexact.c,
>>>       gcc.target/i386/sse2-rint-inline-1.c,
>>>       gcc.target/i386/sse2-rint-inline-2.c,
>>>       gcc.target/i386/sse4_1-builtin-fp-int-inexact.c,
>>>       gcc.target/i386/sse4_1-rint-inline.c: New tests.
>>>