This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix vector rsqrt discovery (PR tree-optimization/68501)


Jakub Jelinek <jakub@redhat.com> writes:
> On Sat, Nov 28, 2015 at 09:38:40AM +0100, Jakub Jelinek wrote:
>> On Sat, Nov 28, 2015 at 08:47:18AM +0100, Richard Biener wrote:
>> > On November 27, 2015 8:40:56 PM GMT+01:00, Jakub Jelinek
>> > <jakub@redhat.com> wrote:
>> > >The recent changes where vector sqrt is represented in the IL using
>> > >IFN_SQRT instead of target specific builtins broke the discovery
>> > >of vector rsqrt, as targetm.builtin_reciprocal is called only
>> > >on builtin functions (not internal functions).  Furthermore,
>> > >for internal fns, not only the IFN_* is significant, but also the
>> > >types (modes actually) of the lhs and/or arguments.
>> > >
>> > >This patch adjusts the target hook, so that the backends can just
>> > >inspect
>> > >the call (builtin or internal function), whatever it is.
>> > >
>> > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>> > 
>> > OK.  Though the other option would be to add an optab with
>> > corresponding IFN.
>> 
>> Yeah, I've been thinking about IFN_RSQRT and rsqrt optab, perhaps that is
>> cleaner and the target hook could go away completely.
>
> So, had a look at this, and the only issue I see is that the various
> targetm.builtin_reciprocal implementations start with various fancy
> conditions:
>   if (! (TARGET_SSE_MATH && !optimize_insn_for_size_p ()
>          && flag_finite_math_only && !flag_trapping_math
>          && flag_unsafe_math_optimizations))
>     return NULL_TREE;
> on i?86,
>   if (optimize_insn_for_size_p ())
>     return NULL_TREE;
> on rs6000 and
>   if (flag_trapping_math
>       || !flag_unsafe_math_optimizations
>       || optimize_size
>       || ! (aarch64_tune_params.extra_tuning_flags
>            & AARCH64_EXTRA_TUNE_RECIP_SQRT))
> on aarch64.  The recip pass is only guarded by its gate, which doesn't say
> anything from the above.
> So, shall I move these conditions to the rsqrt<sf,v4sf,v8sf,v2df>2 expanders
> (but not sure if e.g. the tuning flags or !optimize_size or
> !optimize_insn_for_size_p () is appropriate for expander conditions),

The size conditions are the same problem as PR68432.  I'm not sure
whether my series for that PR has been officially rejected or not.
If it has then I'll need to hack around it some other way,
e.g. by having a target hook that says whether an optab should be
used when optimising for size or speed.

Maybe that isn't such a hack given...

> keep the builtin_reciprocal hook (perhaps renamed to builtin_rsqrt)
> for the purpose of this condition and nothing else (i.e. return a
> boolean) and let the rest be determined from the optab, just commit
> the already posted patch, something else?

...I suppose the problem with adding extra conditions to the expander
is that it would break cases where the expander is used for target
built-ins too.

Maybe optabs shouldn't be used for built-ins if the usage conditions
aren't the same.  But if that's fighting too much against existing usage,
the hook "hack" could check these conditions too.

E.g.:

  bool
  targetm.optab_supported_p (int optab_tag, machine_mode mode1,
                             machine_mode mode2, optimization_type opt_type)

where mode1 and mode2 are the optab modes (only one needed for direct
optabs) and where optimization_type is the type from my PR68432 series.

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]