Re: [PATCH] Consolidate cmp benchtests.

[OndÅej BÃlka <neleai at seznam dot cz>]

On Fri, Sep 06, 2013 at 03:49:51PM +0530, Siddhesh Poyarekar wrote:
> On Fri, Sep 06, 2013 at 10:58:38AM +0200, OndÅej BÃlka wrote:
> > Hi,
> > 
> > Same idea with cmp functions.
> > 
> > Again we would need to select more representative inputs as followup.
> > 
> > For strcasecmp we shuold also check inputs consisting of aA to stress
> > case where conversion is needed.
> > 
> > 
> > 	* benchtests/bench-cmp.h: New file.
> > 	* benchtests/bench-memcmp.c: Use benchtests/bench-cmp.h.
> > 	* benchtests/bench-strcasecmp.c: Likewise.
> > 	* benchtests/bench-strcmp.c: Likewise.
> > 	* benchtests/bench-strncasecmp.c: Likewise.
> > 	* benchtests/bench-strncmp.c: Likewise.
> > 
> > ---
> >  benchtests/bench-cmp.h         |  112 ++++++++++++++++++++
> >  benchtests/bench-memcmp.c      |  159 +---------------------------
> >  benchtests/bench-strcasecmp.c  |  158 +---------------------------
> >  benchtests/bench-strcmp.c      |  225 ++--------------------------------------
> >  benchtests/bench-strncasecmp.c |  189 +--------------------------------
> >  benchtests/bench-strncmp.c     |  221 +--------------------------------------
> >  6 files changed, 138 insertions(+), 926 deletions(-)
> >  create mode 100644 benchtests/bench-cmp.h
> > 
> > diff --git a/benchtests/bench-cmp.h b/benchtests/bench-cmp.h
> 
> Please call this bench-cmp.c since the contents are not merely
> declarations.
> 
ok
> > +    {
> > +      srcs[i][len] = 0;
> > +      CALL2 (impl, dsts[i], srcs[i], len);
> 
> Wouldn't it be nicer to have a more descriptive name, like CPY_FN or
> similar?
> 
On second thought we could use pattern

#if defined AS_STRCMP || defined AS_STRCASECMP
   CALL (impl, dsts[i], srcs[i]);
#else
   CALL (impl, dsts[i], srcs[i], len);
#endif

> > -simple_wmemcmp (const wchar_t *s1, const wchar_t *s2, size_t n)
> > -{
> > -  int ret = 0;
> > -  /* Warning!
> > -	wmemcmp has to use SIGNED comparison for elements.
> > -	memcmp has to use UNSIGNED comparison for elemnts.
> > -  */
> > -  while (n-- && (ret = *s1 < *s2 ? -1 : *s1 == *s2 ? 0 : 1) == 0) {s1++; s2++;}
> > -  return ret;
> > -}
> 
> Why have you dropped these?  Also, you've dropped the wide mode
> function tests.  We don't use them right now because I missed copying
> those tests over from wcsmbs, but I think we should add them here.
>
These are not that useful and tend to consume most of running time. A
more appropriate would be compile implementation from string/function.c
which would be separate patch.

I dropped wide characters as dead code. Adding support should be easy,
just changing char to CHAR and adding appropriate include files.