On Wed, 2014-06-04 at 10:04 +0200, Torsten Bögershausen wrote:
[snip discussion of compiler flags; I'll look into a cpuid approach]

> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -668,6 +668,28 @@ void git_qsort(void *base, size_t nmemb, size_t size,
> >  #endif
> >  #endif
> >  
> > +#ifndef NO_SSE42
> > +#include <nmmintrin.h>
> > +/*
> > + * Clang ships with a version of nmmintrin.h that's incomplete; if
> > + * necessary, we define the constants that we're going to use.
> > + */
> > +#ifndef _SIDD_UBYTE_OPS
> > +#define _SIDD_UBYTE_OPS                 0x00
> > +#define _SIDD_CMP_EQUAL_ANY             0x00
> > +#define _SIDD_CMP_RANGES                0x04
> > +#define _SIDD_CMP_EQUAL_ORDERED         0x0c
> > +#define _SIDD_NEGATIVE_POLARITY         0x10
> > +#endif
> Why do this defines end up in git-compat-util.h when they are needed by one 
> file?
> (see even below)

Because Junio told me to:
"We would prefer not to add inclusion of any system header files in
random *.c files, as there often are system dependencies (order of
inclusion, definition of feature macros, etc.) we would rather want
to encapsulate in one place, that is git-compat-util.h."

> > --- a/refs.c
> > +++ b/refs.c
> > @@ -24,6 +24,25 @@ static unsigned char refname_disposition[256] = {
> >     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
> >  };
> >  
> > +static int check_refname_component_trailer(const char *cp, const char 
> > *refname, int flags)
> > +{
> > +   if (cp == refname)
> > +           return 0; /* Component has zero length. */
> > +   if (refname[0] == '.') {
> > +           if (!(flags & REFNAME_DOT_COMPONENT))
> > +                   return -1; /* Component starts with '.'. */
> > +           /*
> > +            * Even if leading dots are allowed, don't allow "."
> > +            * as a component (".." is prevented by a rule above).
> > +            */
> > +           if (refname[1] == '\0')
> > +                   return -1; /* Component equals ".". */
> > +   }
> > +   if (cp - refname >= 5 && !memcmp(cp - 5, ".lock", 5))
> > +           return -1; /* Refname ends with ".lock". */
> > +   return cp - refname;
> > +}
> > +
> >  /*
> >   * Try to read one refname component from the front of refname.
> >   * Return the length of the component found, or -1 if the component is
> > @@ -37,7 +56,7 @@ static unsigned char refname_disposition[256] = {
> >   * - it ends with ".lock"
> >   * - it contains a "\" (backslash)
> >   */
> > -static int check_refname_component(const char *refname, int flags)
> > +static int check_refname_component_1(const char *refname, int flags)
> The name check_refname_component_1() doesn't tell too much,
> (check_refname_component_sse42()  or check_refname_component_nonsse42() say 
> more)

I'll go with "_bytewise", since that's how it works.

> can I suggest to move all SSE code out to a file under compat/,
> like compat/refs_sse42.c, or something similar ?

Since this is a relatively small section of code, I think that would be
overkill.  Does anyone else have an opinion?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to