On Wed, 23 Aug 2017, Uros Bizjak wrote:
> On Wed, Aug 23, 2017 at 12:18 PM, Richard Biener <rguent...@suse.de> wrote:
> > On Wed, 23 Aug 2017, Uros Bizjak wrote:
> >
> >> On Wed, Aug 23, 2017 at 11:55 AM, Richard Biener <rguent...@suse.de> wrote:
> >> > On Wed, 23 Aug 2017, Uros Bizjak wrote:
> >> >
> >> >> On Wed, Aug 23, 2017 at 11:40 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> >> >> > On Wed, Aug 23, 2017 at 11:22 AM, Richard Biener <rguent...@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 <rguent...@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> > 21284 (AG Nuernberg)
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
