diff mbox

uClibc: ARC: enable generic string implementations

Message ID 1415616842-7640-1-git-send-email-abrodkin@synopsys.com
State Superseded
Headers show

Commit Message

Alexey Brodkin Nov. 10, 2014, 10:54 a.m. UTC
With enabled generic optimized string routines we see significant
improvement of some LMbench results.

For example:

Comments

Thomas Petazzoni Nov. 10, 2014, 12:50 p.m. UTC | #1
Dear Alexey Brodkin,

On Mon, 10 Nov 2014 13:54:02 +0300, Alexey Brodkin wrote:
> With enabled generic optimized string routines we see significant
> improvement of some LMbench results.
> 
> For example:
> ===========
> libc bcopy unaligned
> ...
> 8.39 - 6.76  (UCLIBC_HAS_STRING_GENERIC_OPT=no)
> 8.39 - 23.86 (UCLIBC_HAS_STRING_GENERIC_OPT=yes)
> ===========
> 
> Since I've only did measurements on ARC and it has been ebabled for ARC
> for years I'm sure it doesn't break anything in either during toolchian
> building or in run-time.
> 
> That's why I'm keeping it as arch-specific change.
> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Anton Kolesov <anton.kolesov@synopsys.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  package/uclibc/uclibc.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
> index 419a4d1..3412efd 100644
> --- a/package/uclibc/uclibc.mk
> +++ b/package/uclibc/uclibc.mk
> @@ -66,6 +66,7 @@ ifeq ($(UCLIBC_TARGET_ARCH),arc)
>  UCLIBC_ARC_TYPE = CONFIG_$(call qstrip,$(BR2_UCLIBC_ARC_TYPE))
>  define UCLIBC_ARC_TYPE_CONFIG
>  	$(call KCONFIG_ENABLE_OPT,$(UCLIBC_ARC_TYPE),$(@D)/.config)
> +	$(call KCONFIG_ENABLE_OPT,UCLIBC_HAS_STRING_GENERIC_OPT,$(@D)/.config)

I personally don't like to see such a change done in an architecture
specific way, if the feature is not architecture specific. So we should
evaluate if this works on other architectures, see what is the binary
footprint for this feature, and then decide if we want to enable it by
default in our uClibc configurations.

Thomas
Alexey Brodkin Nov. 12, 2014, 10:55 a.m. UTC | #2
Hi Thomas,

On Mon, 2014-11-10 at 13:50 +0100, Thomas Petazzoni wrote:
> Dear Alexey Brodkin,
> I personally don't like to see such a change done in an architecture
> specific way, if the feature is not architecture specific. So we should
> evaluate if this works on other architectures, see what is the binary
> footprint for this feature, and then decide if we want to enable it by
> default in our uClibc configurations.

I agree that it's better to not have this kind of generic options
selected by architectures.

But the point is I don't have any real hardware except ARC and ARM
boards (Cubieboard2 & Wandboard Quad).

For ARC I do see improvements and still stable behavior of toolchain and
target binaries.

For ARM I don't see any difference in LMbench because most if not all
string routines are already implemented in assembly, sio the option in
question makes no sense.

But for other architectures like BlackFin, PPC, MIPS difference could be
in both performance and stability but this is something I cannot test.

So volunteers are more than welcome.

But what if nobody else cares?

-Alexey
Thomas Petazzoni Nov. 12, 2014, 12:33 p.m. UTC | #3
Dear Alexey Brodkin,

On Wed, 12 Nov 2014 10:55:22 +0000, Alexey Brodkin wrote:

> I agree that it's better to not have this kind of generic options
> selected by architectures.
> 
> But the point is I don't have any real hardware except ARC and ARM
> boards (Cubieboard2 & Wandboard Quad).
> 
> For ARC I do see improvements and still stable behavior of toolchain and
> target binaries.
> 
> For ARM I don't see any difference in LMbench because most if not all
> string routines are already implemented in assembly, sio the option in
> question makes no sense.
> 
> But for other architectures like BlackFin, PPC, MIPS difference could be
> in both performance and stability but this is something I cannot test.
> 
> So volunteers are more than welcome.
> 
> But what if nobody else cares?

If nobody cares, then we just enable the option. And people will
complain when/if it breaks for them.

Thomas
Alexey Brodkin Nov. 12, 2014, 1:54 p.m. UTC | #4
Hi Peter,

On Wed, 2014-11-12 at 13:33 +0100, Thomas Petazzoni wrote:
> Dear Alexey Brodkin,
> 
> On Wed, 12 Nov 2014 10:55:22 +0000, Alexey Brodkin wrote:
> 
> > I agree that it's better to not have this kind of generic options
> > selected by architectures.
> > 
> > But the point is I don't have any real hardware except ARC and ARM
> > boards (Cubieboard2 & Wandboard Quad).
> > 
> > For ARC I do see improvements and still stable behavior of toolchain and
> > target binaries.
> > 
> > For ARM I don't see any difference in LMbench because most if not all
> > string routines are already implemented in assembly, sio the option in
> > question makes no sense.
> > 
> > But for other architectures like BlackFin, PPC, MIPS difference could be
> > in both performance and stability but this is something I cannot test.
> > 
> > So volunteers are more than welcome.
> > 
> > But what if nobody else cares?
> 
> If nobody cares, then we just enable the option. And people will
> complain when/if it breaks for them.

So should I wait for some time or it's time to send a patch that enables
the option for everybody in uClibc-*.config?

-Alexey
Thomas Petazzoni Nov. 12, 2014, 2:18 p.m. UTC | #5
Dear Alexey Brodkin,

On Wed, 12 Nov 2014 13:54:11 +0000, Alexey Brodkin wrote:

> > If nobody cares, then we just enable the option. And people will
> > complain when/if it breaks for them.
> 
> So should I wait for some time or it's time to send a patch that enables
> the option for everybody in uClibc-*.config?

I'd say yes. The uClibc Config.in help text for this option recommends
to enable it, and it defaults to y, so I think it's pretty safe.

Regarding ARC, I see that there are arch-specific optimized string
functions in libc/string/arc/, written in assembly. So by enabling
GENERIC_OPT, what you gained is that you have optimized versions for
those functions that don't have an ARC-specific assembly implementation?

Best regards,

Thomas
Alexey Brodkin Nov. 12, 2014, 2:42 p.m. UTC | #6
Hi Thomas,

On Wed, 2014-11-12 at 15:18 +0100, Thomas Petazzoni wrote:
> Dear Alexey Brodkin,
> 
> On Wed, 12 Nov 2014 13:54:11 +0000, Alexey Brodkin wrote:
> 
> > > If nobody cares, then we just enable the option. And people will
> > > complain when/if it breaks for them.
> > 
> > So should I wait for some time or it's time to send a patch that enables
> > the option for everybody in uClibc-*.config?
> 
> I'd say yes. The uClibc Config.in help text for this option recommends
> to enable it, and it defaults to y, so I think it's pretty safe.

Ok so will do it shortly.

> Regarding ARC, I see that there are arch-specific optimized string
> functions in libc/string/arc/, written in assembly. So by enabling
> GENERIC_OPT, what you gained is that you have optimized versions for
> those functions that don't have an ARC-specific assembly implementation?

Absolutely correct, we're getting better results for functions not
implemented in ASM.

-Alexey
diff mbox

Patch

===========
libc bcopy unaligned
...
8.39 - 6.76  (UCLIBC_HAS_STRING_GENERIC_OPT=no)
8.39 - 23.86 (UCLIBC_HAS_STRING_GENERIC_OPT=yes)
===========

Since I've only did measurements on ARC and it has been ebabled for ARC
for years I'm sure it doesn't break anything in either during toolchian
building or in run-time.

That's why I'm keeping it as arch-specific change.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Anton Kolesov <anton.kolesov@synopsys.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 package/uclibc/uclibc.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
index 419a4d1..3412efd 100644
--- a/package/uclibc/uclibc.mk
+++ b/package/uclibc/uclibc.mk
@@ -66,6 +66,7 @@  ifeq ($(UCLIBC_TARGET_ARCH),arc)
 UCLIBC_ARC_TYPE = CONFIG_$(call qstrip,$(BR2_UCLIBC_ARC_TYPE))
 define UCLIBC_ARC_TYPE_CONFIG
 	$(call KCONFIG_ENABLE_OPT,$(UCLIBC_ARC_TYPE),$(@D)/.config)
+	$(call KCONFIG_ENABLE_OPT,UCLIBC_HAS_STRING_GENERIC_OPT,$(@D)/.config)
 endef
 endif # arc