(patch) hpjyg09: bcache optimizations

[Andrew Cagney]

Jimmy Guo wrote:
> 
> ***
> Patch dependency: hpjyg05 (config/pa/tm-hppa.h)
> ***
> 
> Couple of bcache.c optimizations:
> 
> 1) Introduction and use of inlined hash function, and attempt to reduce
>    the memcmp call.

Um,

If the bcache_hash code really is a performance hit can a static INLINE
C function be added to the header file instead of the macro?  My
experience with simulators taught me that complex macro's should be
avoided like the plague - make it debuggable and then think about making
it fast.

On to the actual patch:

	o	The move to add a ``bcache_'' prefix (and fix some
		name space problems is definitly good.

	o	Is the change duplicating the hash code -
		BCACHE_HASH and bcache.c:hash()?  I think
		it would be safer if GDB only contained
		one implementation of that code (even if it was
		in a macro).  hash() could use BCACHE_HASH.

	o	rather than adding:

+       /* CM: Use the inlined hash function if requested. */
+ #ifdef BCACHE_USE_INLINED_HASH
+       BCACHE_HASH (hashval, bytes, count);
+ #else
        hashval = hash (bytes, count);
+ #endif

		I suspect that it will be easier/cleaner to
		just provide a default BCACHE_HASH definition:
		(HASHVAL) = hash ((BYTES), (COUNT)).


	o	I'm in too minds over the first argument
		(HASHVAL). I recently spent time eliminating
		several really bad macro definitions that
		abused the fact that they had direct access to
		arguments - changing them to multi-arch functions
		was, er, non-trivial.

		However, in this case, the rational and use is
		clean.

Just 1.26c worth,

	Enjoy,
		Andrew


> + /* CM: The hash method should exactly match the function defined below. */
> +
> + #define BCACHE_HASH(retval, bytes, temp_count) \
> +   { \
> +     int bcache_hash_count = (temp_count); \
> +     unsigned int bcache_hash_len; \
> +     unsigned long bcache_hash_hashval; \
> +     unsigned int bcache_hash_c; \
> +     const unsigned char *bcache_hash_data = (bytes); \
> +     \
> +     bcache_hash_hashval = 0; \
> +     bcache_hash_len = 0; \
> +     while (bcache_hash_count-- > 0) \
> +       { \
> +         bcache_hash_c = *bcache_hash_data++; \
> +         bcache_hash_hashval += bcache_hash_c + (bcache_hash_c << 17); \
> +         bcache_hash_hashval ^= bcache_hash_hashval >> 2; \
> +         ++bcache_hash_len; \
> +       } \
> +     bcache_hash_hashval += bcache_hash_len + (bcache_hash_len << 17); \
> +     bcache_hash_hashval ^= bcache_hash_hashval >> 2; \
> +     (retval) = (bcache_hash_hashval % BCACHE_HASHSIZE); \
> +   }