Re: [Qemu-devel] [PATCH] Memory API bugfix - abolish addrrrange_end()

[David Gibson <david@gibson.dropbear.id.au>]

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