Re: [PATCH] Fix PR81921

[Richard Biener <rguenther at suse dot de>]

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?

@@ -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)