Re: [Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.
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. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.
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. > Your patch is probably better, though. I never understood the point of > pack/unpacking these. > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.
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 Your patch is probably better, though. I never understood the point of pack/unpacking these. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.
This seems perfectly fine to me. For what it's worth: Reviewed-by: Dylan Baker Quoting Matt Turner (2016-02-04 17:48:00) > 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: > -- > 2.4.10 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] nir: Handle large unsigned values in opt_algebraic.
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: -- 2.4.10 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev