From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83438 invoked by alias); 28 May 2015 17:25:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 83421 invoked by uid 89); 28 May 2015 17:25:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,SPF_NEUTRAL autolearn=no version=3.3.2 X-HELO: popelka.ms.mff.cuni.cz Date: Thu, 28 May 2015 17:59:00 -0000 From: =?utf-8?B?T25kxZllaiBCw61sa2E=?= To: Joseph Myers Cc: libc-alpha@sourceware.org Subject: Re: [RFC PATCH 4/3] Fix previous patch and add header. Message-ID: <20150528172513.GA4299@domone> References: <20150526173150.GA26817@domone> <20150526181035.GA27596@domone> <20150526185534.GA28344@domone> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-SW-Source: 2015-05/txt/msg00797.txt.bz2 On Thu, May 28, 2015 at 05:17:22PM +0000, Joseph Myers wrote: > On Tue, 26 May 2015, Ondřej Bílka wrote: > > > diff --git a/string/common.h b/string/common.h > > new file mode 100644 > > index 0000000..84f9767 > > --- /dev/null > > +++ b/string/common.h > > @@ -0,0 +1,35 @@ > > +#include > > All files should start with a comment that first describes the file, then > has the copyright and license notice. > > > +/* Use vector arithmetic tricks. Idea is to take expression works on > > + unsigned byte and evaluates 0 for nozero byte and nonzero on zero byte. > > + Our expression is ((s + 127) ^ (~s)) & 128 > > + Now we evaluate this expression on each byte in parallel and on first > > + nonzero byte our expression will have nonzero value. */ > > I think a more detailed, multi-paragraph comment carefully explaining the > algorithm used (and why and how it works) would be better. Like the one > discussed in bug 5806, but of course without the mistake discussed there. > > The principle of a common header with this logic is a good one - hopefully > if it gets used everywhere this can resolve bug 5806 by eliminating all > copies of the old comment. > > (The patch submission should carefully discuss how the algorithm relates > to the ones discussed in bug 5806 / 5807 or used in existing code. But > that's part of justifying the patches rather than for the comments in the > new code.) > > > +static __always_inline > > +unsigned long int > > +contains_zero (unsigned long int s) > > +{ > > + return ((s + add) ^ ~s) & high_bits; > > Instead of using unsigned long int, architectures should be able to > configure the type used. I expect that for ilp32 configurations on 64-bit > architectures, where registers are 64-bit but unsigned long int is 32-bit, > such as x32 and MIPS n32, it will be better to use unsigned long long int. > in next iteration I would add typedef and archs would use it from header. Perhaps something like. #ifndef VECTOR_INT # define VECTOR_INT unsigned long int #endif typedef VECTOR_INT vector_int; > > +#define CROSS_PAGE(x, n) (((uintptr_t)x) % 4096 >= 4096 - n) > > It might also make sense for the use of 4096 here to be configurable by > architectures as well. > Could but it won't likely make noticable difference as you cross page with probability 0.8% for uniformly random address.