From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFP4n-0004zf-4i for qemu-devel@nongnu.org; Sun, 16 Oct 2011 07:40:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RFP4l-00049l-6b for qemu-devel@nongnu.org; Sun, 16 Oct 2011 07:40:21 -0400 Received: from ozlabs.org ([203.10.76.45]:34361) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFP4k-00047Z-Er for qemu-devel@nongnu.org; Sun, 16 Oct 2011 07:40:19 -0400 Date: Sun, 16 Oct 2011 22:40:11 +1100 From: David Gibson Message-ID: <20111016114011.GG4580@truffala.fritz.box> References: <1318387026-21569-1-git-send-email-david@gibson.dropbear.id.au> <4E9AAA33.3010806@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E9AAA33.3010806@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Memory API bugfix - abolish addrrrange_end() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org On Sun, Oct 16, 2011 at 11:56:03AM +0200, Avi Kivity wrote: > On 10/12/2011 04:37 AM, David Gibson wrote: [snip] > > static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2) > > { > > - int64_t start = MAX(r1.start, r2.start); > > - /* off-by-one arithmetic to prevent overflow */ > > - int64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1); > > - return addrrange_make(start, end - start + 1); > > + if (r1.start <= r2.start) { > > + return addrrange_make(r2.start, > > + MIN(r2.size, r1.size - (r2.start - r1.start))); > > + } else { > > + return addrrange_make(r1.start, > > + MIN(r1.size, r2.size - (r1.start - r2.start))); > > + } > > } > > > > This is pretty ugly. A little, but it's correct... > > struct CoalescedMemoryRange { > > @@ -201,7 +202,8 @@ static void flatview_destroy(FlatView *view) > > > > static bool can_merge(FlatRange *r1, FlatRange *r2) > > { > > - return addrrange_end(r1->addr) == r2->addr.start > > + assert (r1->addr.start < r2->addr.start); > > So is this, to a lesser extent. Well, the assert is optional; I basically just put that in to document the assumptions going into this function. > Let me see if I can work up a synthetic int128 type. So.. you think replacing every single basic arithmetic operations with calls to implement the synthetic type, _and_ imposing the resulting overhead is _less_ ugly than some slightly fiddly re-ordering of operations? Seriously? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson