Re: Ping: [Patch] aarch64: Thunderx specific memcpy and memmove

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 23/05/17 21:12, Ellcey, Steve wrote:
> Wilco and Szabolcs,
> 
> 
> Do you have any objection to me going ahead and checking in this patch?
> 

you can commit the patch, however

- we are still looking at posting an updated generic memcpy,
it just takes longer to get it through than expected, when
that happens the thunderx specific memcpy will be reconsidered
and may get removed. (we try to do it in this release cycle)

- i don't know if you plan to make more changes to the thunderx
memcpy, if prefetching is the only change then it's likely that
we can agree on a generic version that's good enough.  if you
do plan to make further changes, then keep in mind that we try
to have same/similar generic memcpy across c runtimes and if
your change is good for generic we might not be able to use the
code outside of glibc (so newlib, bionic, freebsd,.. memcpy
would diverge)

- non-thunderx systems are affected: static linked code using
memcpy will start to go through an indirection (iplt) instead
of direct call. if there are complaints about it or other ifunc
related issues come up, then again we will have to reconsider it.

so the patch can go in with an understanding that it may go out.

> 
> Steve Ellcey
> 
> sellcey@cavium.com
> 
> 
> 
> ---------------------------------------------------------------------------------------------------------------
> *From:* Siddhesh Poyarekar <siddhesh@gotplt.org>
> *Sent:* Friday, May 19, 2017 12:41 AM
> *To:* Ellcey, Steve; Szabolcs Nagy; libc-alpha
> *Cc:* nd@arm.com
> *Subject:* Re: Ping: [Patch] aarch64: Thunderx specific memcpy and memmove
>  
> On Wednesday 10 May 2017 03:15 AM, Steve Ellcey wrote:
>> That sounds reasonable to me.  Here is a patch that contains a separate
>> memcpy_thunderx implementation.  I still have some (minor) changes to
>> the generic memcpy.S file.  One change is to use macros for the
>> function names so that the generic multiarch memcpy can include the
>> standard non-multiarch version.  The other is to change a couple of
>> internal labels to external labels.  This change isn't absolutely
>> necessary but it is helpful in the thunderx memcpy where the branches
>> are slightly different and I would like to keep the thunderx memcpy and
>> the generic memcpy as similar as possible so that when a change happens
>> in one or the other it is easy to compare the two versions.  I don't
>> believe using different label types affects the generated code at all
>> and personally, I find named labels easier to read than the internal
>> numbered labels.  Being able to compare the two memcpy's is also why I
>> kept the THUNDERX ifdef in memcpy_thunderx.S even though it is always
>> defined there, so that the intended differences are explicit when
>> comparing the two versions of memcpy.
>> 
>> Tested on the top-of-tree sources with no regressions.
> 
> The patch looks fine.  Please coordinate with Szabolcs and Wilco on the
> way forward.  You could either commit now and let Wilco rebase on top of
> your changes or wait till he is done with his analysis and then figure
> out the next step.
> 
> Siddhesh
> 
> PS: You don't really need the USE_THUNDERX macros in your memcpy
> anymore, but it's not a big deal if that is what you prefer to keep
> track of generic memcpy changes since you own that code.