This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR81921
On Wed, 23 Aug 2017, Uros Bizjak wrote:
> On Wed, Aug 23, 2017 at 12:18 PM, Richard Biener <rguenther@suse.de> wrote:
> > On Wed, 23 Aug 2017, Uros Bizjak wrote:
> >
> >> On Wed, Aug 23, 2017 at 11:55 AM, Richard Biener <rguenther@suse.de> wrote:
> >> > On Wed, 23 Aug 2017, Uros Bizjak wrote:
> >> >
> >> >> On Wed, Aug 23, 2017 at 11:40 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> >> >> > On Wed, Aug 23, 2017 at 11:22 AM, Richard Biener <rguenther@suse.de> wrote:
> >> >> >>> > /* If the machine does not have a case insn that compares the bounds,
> >> >> >>> > Index: gcc/config/i386/i386.c
> >> >> >>> > ===================================================================
> >> >> >>> > --- gcc/config/i386/i386.c (revision 251275)
> >> >> >>> > +++ gcc/config/i386/i386.c (working copy)
> >> >> >>> > @@ -7369,7 +7369,8 @@ ix86_valid_target_attribute_tree (tree a
> >> >> >>> > || opts->x_target_flags != def->x_target_flags
> >> >> >>> > || option_strings[IX86_FUNCTION_SPECIFIC_ARCH]
> >> >> >>> > || option_strings[IX86_FUNCTION_SPECIFIC_TUNE]
> >> >> >>> > - || enum_opts_set.x_ix86_fpmath)
> >> >> >>> > + || enum_opts_set.x_ix86_fpmath
> >> >> >>> > + || !TARGET_64BIT_P (opts->x_ix86_isa_flags))
> >> >> >>>
> >> >> >>> You should use (!TARGET_64BIT_P ... && TARGET_SSE_P ...) to match the
> >> >> >>> condition in the special fpmath processing part. (BTW: The comment in
> >> >> >>> that part is wrong, we need SSE, not sse2 on 32-bit targets to use SSE
> >> >> >>> math.)
> >> >> >>
> >> >> >> Hmpf, with the change I again run into the libgo bootstrap fail
> >> >> >> (appearantly my reduced testcase is incomplete)
> >> >> >>
> >> >> >> libtool: compile: /abuild/rguenther/obj2/./gcc/xgcc
> >> >> >> -B/abuild/rguenther/obj2/./gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/
> >> >> >> -B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem
> >> >> >> /usr/local/x86_64-pc-linux-gnu/include -isystem
> >> >> >> /usr/local/x86_64-pc-linux-gnu/sys-include -m32 -DHAVE_CONFIG_H -I.
> >> >> >> -I/space/rguenther/src/svn/trunk2/libgo -I
> >> >> >> /space/rguenther/src/svn/trunk2/libgo/runtime
> >> >> >> -I/space/rguenther/src/svn/trunk2/libgo/../libffi/include
> >> >> >> -I../libffi/include -pthread -fexceptions -fnon-call-exceptions
> >> >> >> -fplan9-extensions -fsplit-stack -Wall -Wextra -Wwrite-strings -Wcast-qual
> >> >> >> -Werror -minline-all-stringops -D_GNU_SOURCE -D_LARGEFILE_SOURCE
> >> >> >> -D_FILE_OFFSET_BITS=64 -I /space/rguenther/src/svn/trunk2/libgo/../libgcc
> >> >> >> -I /space/rguenther/src/svn/trunk2/libgo/../libbacktrace -I
> >> >> >> ../../../gcc/include -g -O2 -m32 -MT aeshash.lo -MD -MP -MF
> >> >> >> .deps/aeshash.Tpo -c
> >> >> >> /space/rguenther/src/svn/trunk2/libgo/runtime/aeshash.c -fPIC -DPIC -o
> >> >> >> .libs/aeshash.o
> >> >> >> In file included from
> >> >> >> /space/rguenther/src/svn/trunk2/libgo/runtime/aeshash.c:17:0:
> >> >> >> /space/rguenther/src/svn/trunk2/libgo/runtime/aeshash.c: In function
> >> >> >> ‘aeshashbody’:
> >> >> >> /abuild/rguenther/obj2/./gcc/include/emmintrin.h:700:1: error: inlining
> >> >> >> failed in call to always_inline ‘_mm_loadu_si128’: target specific option
> >> >> >> mismatch
> >> >> >> _mm_loadu_si128 (__m128i_u const *__P)
> >> >> >> ^~~~~~~~~~~~~~~
> >> >> >> /space/rguenther/src/svn/trunk2/libgo/runtime/aeshash.c:387:11: note:
> >> >> >> called from here
> >> >> >> mseed ^= _mm_loadu_si128(aeskeysched.__values);
> >> >> >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> >>
> >> >> >> that appens even with making the if a if (1). The check we run into
> >> >> >> is somehow still
> >> >> >>
> >> >> >> else if (caller_opts->x_ix86_fpmath != callee_opts->x_ix86_fpmath)
> >> >> >> ret = false;
> >> >> >>
> >> >> >> ah ...
> >> >> >>
> >> >> >> #ifndef __SSE__
> >> >> >> #pragma GCC push_options
> >> >> >> #pragma GCC target("sse")
> >> >> >> #define __DISABLE_SSE__
> >> >> >> #endif /* __SSE__ */
> >> >> >>
> >> >> >> but on x86_64 when building with -m32 we _do_ have __SSE__ set so
> >> >> >> the intrinsics get target_option_default_node which doesn't have
> >> >> >> fmpath=sse. So x86_64 -m32 -march=i586 enables SSE math for
> >> >> >> the intrinsics while x86_64 -m32 [-march=x86-64] does not...
> >> >> >
> >> >> > IMO target_default_node should already enable fpmath=sse. When SSE is
> >> >> > available, fpmath should be set to SSE, unless there is fpmath option
> >> >> > specified.
> >> >>
> >> >> Following part from ix86_option_override_internal looks a bit mismatched:
> >> >>
> >> >> --cut here--
> >> >> /* For all chips supporting SSE2, -mfpmath=sse performs better than
> >> >> fpmath=387. The second is however default at many targets since the
> >> >> extra 80bit precision of temporaries is considered to be part of ABI.
> >> >> Overwrite the default at least for -ffast-math.
> >> >> TODO: -mfpmath=both seems to produce same performing code with bit
> >> >> smaller binaries. It is however not clear if register allocation is
> >> >> ready for this setting.
> >> >> Also -mfpmath=387 is overall a lot more compact (bout 4-5%) than SSE
> >> >> codegen. We may switch to 387 with -ffast-math for size optimized
> >> >> functions. */
> >> >> else if (fast_math_flags_set_p (&global_options)
> >> >> && TARGET_SSE2_P (opts->x_ix86_isa_flags))
> >> >> opts->x_ix86_fpmath = FPMATH_SSE;
> >> >> --cut here--
> >> >>
> >> >> We probably have to set fpmath to SSE also for (!TARGET_64BIT_P ... &&
> >> >> TARGET_SSE_P ...).
> >> >
> >> > The comment says we can't because 387 math is considered part of the ABI.
> >>
> >> Then ix86_valid_target_attribute_tree needs to set fpmath in the same
> >> way to avoid fpmath mismatch, probably using TARGET_FPMATH_DEFAULT.
> >
> > Or keep it at the global opts level?
>
> Yep, ix86_option_override_internal will later do its magic. Needs some
> testing, though.
That seems to work. It might still end up rejecting code that was
accepted earlier in case the phase of the moon aligned. We can
improve in the comparison with (hack alert)
- else if (caller_opts->x_ix86_fpmath != callee_opts->x_ix86_fpmath)
- ret = false;
+ else if (caller_opts->x_ix86_fpmath != callee_opts->x_ix86_fpmath
+ && ipa_fn_summaries->get (cgraph_node::get (callee))->fp_expressions)
+ ret = false;
thus ignore the fpmath setting in the callee if it doesn't use any FP
(also fixes the libgo build). Hopefully IPA summaries are available
in all caller contexts but changing the target hook interface would
be nicer (passing cgraph nodes and fn summaries).
This would be a change that can't regress anything (besides ICEing)
and likely would fix most of the intrinsic header issues apart from
open-coded vector math.
Honza?
> Uros.
>
> > @@ -7398,16 +7400,6 @@ ix86_valid_target_attribute_tree (tree a
> > /* If fpmath= is not set, and we now have sse2 on 32-bit, use it.
> > */
> > if (enum_opts_set.x_ix86_fpmath)
> > opts_set->x_ix86_fpmath = (enum fpmath_unit) 1;
> > - else if (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
> > - && TARGET_SSE_P (opts->x_ix86_isa_flags))
> > - {
> > - if (TARGET_80387_P (opts->x_target_flags))
> > - opts->x_ix86_fpmath = (enum fpmath_unit) (FPMATH_SSE
> > - | FPMATH_387);
> > - else
> > - opts->x_ix86_fpmath = (enum fpmath_unit) FPMATH_SSE;
> > - opts_set->x_ix86_fpmath = (enum fpmath_unit) 1;
> > - }
> >
> > /* Do any overrides, such as arch=xxx, or tune=xxx support. */
> > bool r = ix86_option_override_internal (false, opts, opts_set);
> >
> >
> >> Uros.
> >>
> >>
> >
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)