From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: "\"Øyvind A. Holm\"" <sunny@sunbase.org>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Thomas Rast <trast@student.ethz.ch>
Subject: [PATCH 3/3] xdiff: import new 32-bit version of count_masked_bytes()
Date: Tue, 22 May 2012 22:36:57 +0200 [thread overview]
Message-ID: <4FBBF8E9.7020103@lsrfire.ath.cx> (raw)
In-Reply-To: <CAA787rmJixvyKhubHXZCDVYc=DdVk8_vHsHF6bOsLQ_j=39bGw@mail.gmail.com>
Import the latest 32-bit implementation of count_masked_bytes() from
Linux (arch/x86/include/asm/word-at-a-time.h). It's shorter and avoids
overflows and negative numbers.
This fixes test failures on 32-bit, where negative partial results had
been shifted right using the "wrong" method (logical shift right instead
of arithmetic short right). The compiler is free to chose the method,
so it was only wrong in the sense that it didn't work as intended by us.
Reported-by: Øyvind A. Holm <sunny@sunbase.org>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Does this fix all the warnings, and do the tests pass? I can only
reproduce the "shifting to far to the right" warning..
xdiff/xutils.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 78549e3..9504eae 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -280,19 +280,11 @@ static inline long count_masked_bytes(unsigned long mask)
long a = (REPEAT_BYTE(0x01) / 0xff + 1);
return mask * a >> (sizeof(long) * 7);
} else {
- /*
- * Modified Carl Chatfield G+ version for 32-bit *
- *
- * (a) gives us
- * -1 (0, ff), 0 (ffff) or 1 (ffffff)
- * (b) gives us
- * 0 for 0, 1 for (ff ffff ffffff)
- * (a+b+1) gives us
- * correct 0-3 bytemask count result
- */
- long a = (mask - 256) >> 23;
- long b = mask & 1;
- return a + b + 1;
+ /* Carl Chatfield / Jan Achrenius G+ version for 32-bit */
+ /* (000000 0000ff 00ffff ffffff) -> ( 1 1 2 3 ) */
+ long a = (0x0ff0001 + mask) >> 23;
+ /* Fix the 1 for 00 case */
+ return a & mask;
}
}
--
1.7.10.2
next prev parent reply other threads:[~2012-05-22 20:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-16 23:31 tr/xdiff-fast-hash generates warnings and breaks tests Øyvind A. Holm
2012-05-17 7:11 ` René Scharfe
2012-05-17 9:33 ` Øyvind A. Holm
2012-05-17 16:23 ` Junio C Hamano
2012-05-17 18:40 ` René Scharfe
2012-05-19 14:17 ` Øyvind A. Holm
2012-05-22 20:36 ` [PATCH 1/3] xdiff: avoid compiler warnings with XDL_FAST_HASH on 32-bit machines René Scharfe
2012-05-22 20:36 ` [PATCH 2/3] xdiff: avoid more " René Scharfe
2012-05-23 8:30 ` Thomas Rast
2012-05-22 20:36 ` René Scharfe [this message]
2012-05-25 15:18 ` [PATCH 3/3] xdiff: import new 32-bit version of count_masked_bytes() Øyvind A. Holm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FBBF8E9.7020103@lsrfire.ath.cx \
--to=rene.scharfe@lsrfire.ath.cx \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sunny@sunbase.org \
--cc=trast@student.ethz.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).