Re: [PATCH][AArch64] Implement vsqrt_f64 intrinsic

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

On 17/12/14 00:04, Joseph Myers wrote:
> On Mon, 15 Dec 2014, James Greenhalgh wrote:
>
> > > @@ -22792,6 +22792,12 @@ vsqrtq_f32 (float32x4_t a)
> > >     return __builtin_aarch64_sqrtv4sf (a);
> > >   }
> > >   
> > > +__extension__ static __inline float64x1_t __attribute__ ((__always_inline__))
> > > +vsqrt_f64 (float64x1_t a)
> > > +{
> > > +  return (float64x1_t) { __builtin_sqrt (a[0]) };
> > > +}
> > Hi Kyrill,
> >
> > Does this introduce an implicit need to link against a maths library if
> > we want arm_neon.h to work correctly? If so, I think we need to take a
> > different approach.
> >
> > At O0 I've started to see:
> >
> >    " undefined reference to `sqrt' "
> >
> > When checking a large arm_neon.h testcase.
> >
> > It does seem strange that the mid-end would convert a __builtin_sqrt back
> > to a library call at O0 when the target has an optab for it, so perhaps
> > there is a bug there to go hunt?
> __builtin_sqrt has the same semantics as the sqrt library function.  This
> includes setting errno for negative arguments (other than -0 and -NaN).
> The semantics also include that it's always OK to expand to a call to that
> library function (generally, __builtin_foo may always expand to a call to
> foo, if there is such a library function).

So my first attempt at this patch had created a target builtin 
(__builtin_aarch64_sqrtdf) and used that. Eventually though I went for 
the shorter __builtin_sqrt because I thought we could benefit from the 
tree-level information about the semantics rather than the RTL-level 
expansion that the target-specific builtin would provide.
But if there's a risk for it to expand to a library function call, I 
guess it's better to go with the target builtin. I'll prepare a patch.

Thanks for the explanations,
Kyrill

>