[PATCH 1/3] New generic sincosf

[H.J. Lu]

On Fri, Dec 15, 2017 at 8:34 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Dec 15, 2017 at 8:24 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Fri, 15 Dec 2017, H.J. Lu wrote:
>>
>>> On Fri, Dec 15, 2017 at 7:48 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> > On Fri, 15 Dec 2017, H.J. Lu wrote:
>>> >
>>> >> > glibc is build with -fmerge-all-constants, so constants should already be
>>> >> > shared at link time; making constants hidden would be relevant only for
>>> >> > arrays, not for individual floating-point numbers (and if the individual
>>> >> > constants aren't put in appropriate sections with link-time merging, it's
>>> >> > probably because not doing so is more efficient on a particular
>>> >> > architecture).
>>> >>
>>> >> What do you suggest for x86-64 to avoid array duplication?
>>> >
>>> > Well, a followup patch to refactor the arrays into hidden variables would
>>> > be reasonable once the sincosf patch is in - but it would need
>>> > benchmarking to make sure it doesn't adversely affect performance.
>>>
>>> Do you have any suspicions to indicate that hidden array may be
>>> slower than local one to access?
>>
>> On some architectures, GCC uses section anchors for access to variables
>> defined in the same source file.  In such cases, moving the arrays to
>> being hidden and defined separately means that the address of each array
>> needs to be loaded separately because the offsets between different
>> variables are no longer known when the source files using them are being
>> compiled, whereas if defined in each source file, a function accessing
>> multiple arrays could load the anchor address and them compute all the
>> other addresses with small offsets from it.
>
> This won't show up on x86.
>
>> You should be able to avoid that problem by putting all the data in a
>> single structure, as done in e_exp2f_data.c, rather than separate hidden
>> variables.
>>
>
> Good to know.
>
> I have no objections now.  I will prepare a patch after s_sincosf.c is checked
> in.
>

One comment,  should sysdeps/ieee754/flt-32/s_sincos.h be renamed
to  sysdeps/ieee754/flt-32/s_sincosf.h?

I am enclosing a patch to add sysdeps/ieee754/flt-32/s_sincosf_data.c.
It saves 352 bytes on x86-64.

H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-s_sincosf_data.c.patch
Type: text/x-patch
Size: 5075 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20171215/d4a24040/attachment.bin>