strncpy optimization for PowerPC64

[luporl]

Summary

Update: strncpy.S optimized

Since PowerPC has hardware support for memory alignment, the initial session was removed. Also, the copy-by-byte session (Lstd_byte) was replaced by a faster code.

Performance gain comparing to the former strncpy.S

String size (bytes)	size <= 8	16 <= size <= 32	64 <= size <= 128	256 <= size <= 512	1024 <= size <= 2048
Gain rate	0.17%	0.31%	0.62%	0.37%	0.22%

Performance gain comparing to the original strncpy (linked in libc):

String size (bytes)	size <= 8	16 <= size <= 32	64 <= size <= 128	256 <= size <= 512	1024 <= size <= 2048
Gain rate	0.23%	2.48%	8.30%	25.81%	55.12%

[alexandre.osorio_eldorado.org.br]

Optimizing strncpy.S
Since PowerPC has hardware support for memory alignment, the initial session was removed. Also, the copy-by-byte session (Lstd_byte) was replaced by a faster code.

Related to the former code, the performance gain rates are:

bytesToCopy <= 8 : 0.17%
16 <= bytesToCopy <= 32 : 0.50%
64 <= bytesToCopy <= 128 : 0.60%
256 <= bytesToCopy <= 512 : 0.33%
1024 <= bytesToCopy <= 2048 : 0.19%

[leonardo.bianconi_eldorado.org.br]

Please, Can anyone review this?

[jhibbits]

Can you test on misaligned buffers?

Also, this appears to be the only diff that uses raw numbers for registers, instead of using the '%r*' convention. Can you adjust to match the rest of the style?

[jhibbits]

I think this, and the other related string optimizations, needs revisited regarding alignment. I tested this on a e5500 based system, and glib segfaulted constantly, due to string issues. The problem is that misaligned accesses can cross a page boundary on the last byte(s). If the following page is unmapped, a fault occurs, even if the copy is valid.

[luporl]

• Use %r* instead of raw numbers for registers

[luporl]

Changed register convention.
Still need to investigate and fix the misaligned access issue.

[luporl]

• Handled misalignment issues
• Improved implementation
• Now using memset() for faster zeroing of dst buffer

[luporl]

I consider the strncpy implementation complete now.
Adding ifunc to select the optimized implementation, however, is still not possible, as rtld doesn't support it yet.

[jhibbits]

Have you checked performance on this latest version?

[luporl]

In D15369#456889, @jhibbits wrote:

Have you checked performance on this latest version?

Yes, I have. It is practically the same as that of the last version, except for cases where 'n' is bigger than the string length. In this case there is a big improvement, as the "zero the rest of the buffer" part was being done byte by byte and now memset() is used instead.

[jhibbits]

lib/libc/powerpc64/string/strncpy.S

101 ↗

(On Diff #59760)

Wouldn't we want to zero the remaining if 0 is found earlier?

[luporl]

lib/libc/powerpc64/string/strncpy.S

101 ↗

(On Diff #59760)

Yes, you're right, I've missed this case. I'll fix it.

[luporl]

• [PPC64] strncpy - fix 'dst' not zeroed issue
• Initial ifunc capable strncpy implementation

[luporl]

This last change fixes the previous "dst not always zeroed" issue and adds ifunc support to strncpy, so that the optimized version is selected on ISAs >= 2.05 while others can use the fallback implementation in C.

While it works, I've raise a few questions/issues/points to improve in the following comments.

lib/libc/powerpc64/string/strncpy.c

35–36 ↗

(On Diff #60328)

There should be a better way to reuse the C implementation of strncpy...
Suggestions?

38–39 ↗

(On Diff #60328)

This corresponds to the optimized assembly implementation, that is in another .o file.
I've confirmed that this works, but I think I've read somewhere that ifunc resolvers should choose between functions from the same compilation unit. Does anyone know about such limitation?

lib/libc/string/strncpy.c

49 ↗

(On Diff #60328)

Maybe strncpy() should also be static when included? Or use a reserved identifier (__strncpy_c)?

[luporl]

• [PPC64] strncpy - fix rtld-libc build issue

[luporl]

• [PPC64] strncpy - fix rtld crash

Avoid using ifuncs in rtld.

Although it could be possible to make rtld capable of resolving
its own ifunc relocations, this would add complexity and make
the parts of code executed before ifuncs were relocated dangerous,
as any call to an unresolved ifunc would most probably result in
a crash.

[luporl]

lib/libc/powerpc64/string/Makefile.inc

5 ↗

(On Diff #60393)

This contains the common, C implementation of strncpy().
It is exported as a weak symbol, to make it possible for binaries like rtld to use the non-ifunc version of the function.
It is also made available as __strncpy(), allowing the resolver to reference it.

[luporl]

• Merge branch 'master'

[luporl]

Fix diff

[jhibbits]

lib/libc/powerpc64/string/strncpy_resolver.c

44 ↗

(On Diff #63290)

Wait, where does hwcap come from? It's static in lib/libc/gen/auxv.c.

[bdragon]

This depends on the ifunc support code landing first, of course.

lib/libc/powerpc64/string/strncpy_resolver.c

44 ↗

(On Diff #63290)

Agree. In my current code for the ifunc prototype (which I need to submit real soon now), I have r3 called "cpu_features" and r4 called "cpu_features2", NOT hwcap/hwcap2.

[luporl]

lib/libc/powerpc64/string/strncpy_resolver.c

44 ↗

(On Diff #63290)

Hmm, hwcap comes from my version of the ifunc.h file, from DEFINE_UIFUNC macro.
So, in version of ifunc.h that will be commited it will be called cpu_features instead?

[bdragon]

lib/libc/powerpc64/string/strncpy_resolver.c

44 ↗

(On Diff #63290)

Yeah, I went through a few iterations on it.
Here's a snapshot of what's in my "superglue" integration branch at the moment: https://raw.githubusercontent.com/freebsd/freebsd/c37b3144d0537f0bac9d1ed445c4a2a296954425/sys/powerpc/include/ifunc.h

I'll try and get around to finally getting "RTLD WIP3" fully cleaned up and ready today, that's been on my todo list for too long.

[luporl]

lib/libc/powerpc64/string/strncpy_resolver.c

44 ↗

(On Diff #63290)

Ok, I'll change this part. The other libc optimizations may need this change too, I'll need to check.

Nice, ifunc support in RTLD is the last missing piece blocking libc optimizations and ifunc use in general. Well, that and the ELFv2 switch :)

[luporl]

Fix ifunc resolver parameter name.