Re: [PING][PATCHv3 1/2] aarch64: Hoist ZVA check out of the memset function

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

On 12/10/17 13:57, Siddhesh Poyarekar wrote:
> On Thursday 12 October 2017 04:43 PM, Szabolcs Nagy wrote:
>> since the patch modifies the memset code that runs in
>> most cases, the impact of the change should be well
>> understood before it goes in.
>>
>> in the static linked case it will introduce a plt
>> indirection, i don't expect a big gain except on falkor
>> and there is a risk of regression on other cores, it
>> also makes the code less maintainable.
> 
> Why do you think this gives a gain only on falkor, especially when I
> asserted that it gives a significant gain on falkor *and* mustang?  Why
> do you think having a bunch of branches in live code (that's the bit
> that gets hoisted out) would be beneficial to any core?
> 

there was no testing on real workloads only on
a ubenchmark that has issues outlined by wilco.

it is known that some cores are sensitive to the
exact code sequence in memset and you changed it
without testing the affected cores (cortex-a53),
probably only wilco knows why he wrote the code
exactly the way it is written, so i need evidence
that there is no regression or wilco's approval.

mustang is a devboard with x-gene cores, which is
not representative of the existing aarch64 cores
in use, but even on mustang it's not clear that the
ubenchmark speed up mean improvement in practice.
(string functions are hard to benchmark, there are
many cases and concerns, which is why i'm conservative
about accepting patches to them)

in principle the approach is fine, but it will take
more time to get confident about the patch.