[Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.

[Kenneth Graunke]

On Monday, February 8, 2016 4:01:37 PM PST Ian Romanick wrote:
> On 02/08/2016 01:59 PM, Kenneth Graunke wrote:
> > On Thursday, February 4, 2016 5:48:00 PM PST Matt Turner wrote:
> >> The next patch adds an algebraic rule that uses the constant 0xff00ff00.
> >>
> >> Without this change, the build fails with
> >>
> >>    return hex(struct.unpack('I', struct.pack('i', self.value))[0])
> >>    struct.error: 'i' format requires -2147483648 <= number <= 2147483647
> >>
> >> The hex() function handles integers of any size, and assigning a
> >> negative value to an unsigned does what we want in C. The pack/unpack is
> >> unnecessary (and as we see, buggy).
> >> ---
> >>  src/compiler/nir/nir_algebraic.py | 5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/src/compiler/nir/nir_algebraic.py
> >> b/src/compiler/nir/nir_algebraic.py index 77ad35e..2357b57 100644
> >> --- a/src/compiler/nir/nir_algebraic.py
> >> +++ b/src/compiler/nir/nir_algebraic.py
> >> @@ -102,13 +102,10 @@ class Constant(Value):
> >>        self.value = val
> >>
> >>     def __hex__(self):
> >> -      # Even if it's an integer, we still need to unpack as an unsigned
> >> -      # int.  This is because, without C99, we can only assign to the first
> >> -      # element of a union in an initializer.
> >>        if isinstance(self.value, (bool)):
> >>           return 'NIR_TRUE' if self.value else 'NIR_FALSE'
> >>        if isinstance(self.value, (int, long)):
> >> -         return hex(struct.unpack('I', struct.pack('i', self.value))[0])
> >> +         return hex(self.value)
> >>        elif isinstance(self.value, float):
> >>           return hex(struct.unpack('I', struct.pack('f', self.value))[0])
> >>        else:
> > 
> > FWIW, I sent a patch to fix this on January 19th which went unreviewed:
> > https://lists.freedesktop.org/archives/mesa-dev/2016-January/105387.html
> 
> I was going to R-b it...  After you NAKed the second patch in the series
> I waited for v2.

Oh.  Sorry for the confusion.  That series was actually fine - I just
had a fabs/iabs mixup.  With that fixed, everything worked fine, and I
considered it out for review again.  I suppose I should just re-send it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160208/638c0891/attachment.sig>