Re: [PATCH] Consolidate cmp benchtests.

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Fri, Sep 06, 2013 at 03:00:17PM +0200, OndÅej BÃlka wrote:
> > 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

Sure, but CPY_FN (or COPY_FN or even COPY_FUNCTION) is still more
descriptive than CALL.

> > > -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.

Fine, then remove in the patch that compiles in the implementation
form string/function.c.

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

In general I would prefer it if functionality was not removed till a
replacement is written in.  In that sense, it would be a better idea
to bring in the wcmbs tests first and then update the tests.

Siddhesh