Re: [PATCH, middle-end]: Fix PR78738, unrecognized insn with -ffast-math

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Tue, Dec 13, 2016 at 09:52:40AM +0100, Richard Biener wrote:
> On Sun, Dec 11, 2016 at 5:16 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Fri, Dec 9, 2016 at 11:09 AM, Richard Biener
> > <richard.guenther@gmail.com> wrote:
> >> On Thu, Dec 8, 2016 at 10:44 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> >>> Hello!
> >>>
> >>> Attached patch fixes fall-out from excess-precision improvements
> >>> patch. As shown in the PR, the code throughout the compiler assumes
> >>> FLAG_PRECISION_FAST when flag_unsafe_math_optimizations flag is in
> >>> effect. The patch puts back two lines, removed by excess-precision
> >>> improvements patch.
> >>>
> >>> 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
> >>>
> >>>     PR middle-end/78738
> >>>     * toplev.c (init_excess_precision): Initialize flag_excess_precision
> >>>     to EXCESS_PRECISION_FAST for flag_unsafe_math_optimizations.
> >>>
> >>> testsuite/ChangeLog:
> >>>
> >>> 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
> >>>
> >>>     PR middle-end/78738
> >>>     * gcc.target/i386/pr78738.c: New test.
> >>>
> >>> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> >>>
> >>> OK for mainline?
> >>
> >> Hmm, I think it belongs to set_unsafe_math_optimization_flags instead
> >> (and be consistent if -fexcess-precision was manually specified).
> >>
> >> Also where do we assume connection between -funsafe-math-optimizations
> >> and FLAG_PRECISION_FAST?  We have two flags so we should fix any
> >> user that looks at one but means the other.
> >
> > The failure is caused by the call to ix86_emit_i387_round in
> > "lround<X87MODEF:mode><SWI248x:mode>2" expander. This expander is
> > enabled for x87 math when flag_unsafe_math_optimizations is enabled.
> > In the called ix86_emit_i387_round, DFmode PLUS pattern is manually
> > emitted:
> >
> >   emit_insn (gen_rtx_SET (e2, gen_rtx_PLUS (inmode, e1, half)));
> >
> > but due to  the definition of X87_ENABLE_ARITH, DFmode fadd pattern
> > remains disabled.
> >
> > It is possible to fix the failure by enabling X87_ENABLE_ARITH (and
> > X87_ENABLE_FLOAT) for flag_unsafe_math_optimizations (as is the case
> > in the attached v2 patch), but since gcc-6.x does
> >
> > if (flag_unsafe_math_optimizations)
> >    flag_excess_precision = EXCESS_PRECISION_FAST;
> >
> > I though it was worth mentioning the difference between gcc-6 and
> > gcc-7. Probably, x87 is the only target that cares for it, in which
> > case the attached patch is sufficient.
> 
> I think that patch is the correct correctness fix.
> 
> With respect to GCC 6 vs. GCC 7 behavior I believe it would be more
> reasonable to set EXCESS_PRECISION_FAST by -ffast-math/-Ofast
> rather than from simply -funsafe-math-optimizations (which is an option
> directly controlling "legacy" controlled stuff which should be moved
> under a more specific option umbrella).

Do note that EXCESS_PRECISION_FAST is the default under the GNU dialects,
and we're only in EXCESS_PRECISION_STANDARD in the testcase because of
-std=c99.

That said, relaxing to EXCESS_PRECISION_FAST with -Ofast does make sense
to me.

Thanks,
James