On Wed, 30 May 2007, Nicolas Pitre wrote:
>
> This patch adds a range test to hexval() in order to prevent this. Also
> let's index the hexval_table array directly in get_sha1_hex() using
> explicitly unsigned chars to avoid the range test producing faster
> code.
Please just make hexval() take a "unsigned char" instead, solving the
problem in a manner that makes it much easier for the compiler to realize
that it never needs to sign-extend the value or test the end result..
Ie I think your patch would be better off something like the following
instead (it would be a one-liner, but I agree that marking hexval_table
"const" is also a good idea).
With this, gcc can just generate:
movzbl (%rdi), %eax
movsbl hexval_table(%rax),%edx
movzbl 1(%rdi), %eax
movsbl hexval_table(%rax),%eax
sall $4, %edx
orl %eax, %edx
for the code to generate a byte from two hex characters.
Linus
----
diff --git a/cache.h b/cache.h
index ec85d93..0da7070 100644
--- a/cache.h
+++ b/cache.h
@@ -359,8 +359,8 @@ extern void *map_sha1_file(const unsigned char *sha1, unsigned long *);
extern int has_pack_file(const unsigned char *sha1);
extern int has_pack_index(const unsigned char *sha1);
-extern signed char hexval_table[256];
-static inline unsigned int hexval(unsigned int c)
+extern const signed char hexval_table[256];
+static inline unsigned int hexval(unsigned char c)
{
return hexval_table[c];
}
diff --git a/sha1_file.c b/sha1_file.c
index 12d2ef2..f959fe5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -33,7 +33,7 @@ const unsigned char null_sha1[20];
static unsigned int sha1_file_open_flag = O_NOATIME;
-signed char hexval_table[256] = {
+const signed char hexval_table[256] = {
-1, -1, -1, -1, -1, -1, -1, -1, /* 00-07 */
-1, -1, -1, -1, -1, -1, -1, -1, /* 08-0f */
-1, -1, -1, -1, -1, -1, -1, -1, /* 10-17 */
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
