On Thu, Oct 27, 2016 at 10:06 PM, Ian Romanick <i...@freedesktop.org> wrote: > From: Ian Romanick <ian.d.roman...@intel.com> > > Previously both sources were unsized. This caused problems when the > thing being shifted was 64-bit but the shift count was 32-bit. The > expectation in NIR is that all unsized sources (and destination) will > ultimately have the same size. > > The changes in nir_opt_algebraic.py are to prevent errors like: > > Failed to parse transformation: > 03:12:25 (('extract_i8', 'a', 'b'), ('ishr', ('ishl', 'a', ('imul', > ('isub', 3, 'b'), 8)), 24), 'options->lower_extract_byte') > 03:12:25 Traceback (most recent call last): > 03:12:25 File > "/home/jenkins/workspace/Leeroy_2/repos/mesa/src/compiler/nir/nir_algebraic.py", > line 610, in __init__ > 03:12:25 xform = SearchAndReplace(xform) > 03:12:25 File > "/home/jenkins/workspace/Leeroy_2/repos/mesa/src/compiler/nir/nir_algebraic.py", > line 495, in __init__ > 03:12:25 BitSizeValidator(varset).validate(self.search, self.replace) > 03:12:25 File > "/home/jenkins/workspace/Leeroy_2/repos/mesa/src/compiler/nir/nir_algebraic.py", > line 311, in validate > 03:12:25 validate_dst_class = self._validate_bit_class_up(replace) > 03:12:25 File > "/home/jenkins/workspace/Leeroy_2/repos/mesa/src/compiler/nir/nir_algebraic.py", > line 414, in _validate_bit_class_up > 03:12:25 src_class = self._validate_bit_class_up(val.sources[i]) > 03:12:25 File > "/home/jenkins/workspace/Leeroy_2/repos/mesa/src/compiler/nir/nir_algebraic.py", > line 420, in _validate_bit_class_up > 03:12:25 assert src_class == src_type_bits > 03:12:25 AssertionError
It's pretty great that Jason wrote this validation pass, since otherwise this would've been a rare and hard-to-diagnose failure condition at runtime. Anyways, much better! This patch is Reviewed-by: Connor Abbott <cwabbo...@gmail.com> The commit message on the next patch is out-of-date now, but other than that it seems ok too. I think the only remaining issue is making conversions use implicitly sized types. > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> > Suggested-by: Connor Abbott <cwabbo...@gmail.com> > Cc: Connor Abbott <cwabbo...@gmail.com> > Cc: Jason Ekstrand <ja...@jlekstrand.net> > --- > > I will blame the cold medicine, but when I read Connor's statement, > "legal for one input to be explicitly sized while the other is unsized" > I parsed it more like "legal for one input to be one size while the > other is another sized". The latter is quite a bit more general than > the former. Oops. It's fine, we all make silly mistakes sometimes :) > > src/compiler/nir/nir_opcodes.py | 6 +++--- > src/compiler/nir/nir_opt_algebraic.py | 8 ++++---- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py > index 1825dc3..2af146a 100644 > --- a/src/compiler/nir/nir_opcodes.py > +++ b/src/compiler/nir/nir_opcodes.py > @@ -497,9 +497,9 @@ binop("seq", tfloat32, commutative, "(src0 == src1) ? > 1.0f : 0.0f") # Set on Equ > binop("sne", tfloat32, commutative, "(src0 != src1) ? 1.0f : 0.0f") # Set on > Not Equal > > > -binop("ishl", tint, "", "src0 << src1") > -binop("ishr", tint, "", "src0 >> src1") > -binop("ushr", tuint, "", "src0 >> src1") > +opcode("ishl", 0, tint, [0, 0], [tint, tuint32], "", "src0 << src1") > +opcode("ishr", 0, tint, [0, 0], [tint, tuint32], "", "src0 >> src1") > +opcode("ushr", 0, tuint, [0, 0], [tuint, tuint32], "", "src0 >> src1") > > # bitwise logic operators > # > diff --git a/src/compiler/nir/nir_opt_algebraic.py > b/src/compiler/nir/nir_opt_algebraic.py > index 82d92f4..5e384d9 100644 > --- a/src/compiler/nir/nir_opt_algebraic.py > +++ b/src/compiler/nir/nir_opt_algebraic.py > @@ -341,19 +341,19 @@ optimizations = [ > ('ubfe', 'value', 'offset', 'bits')), > 'options->lower_bitfield_extract'), > > - (('extract_i8', a, b), > + (('extract_i8', a, 'b@32'), > ('ishr', ('ishl', a, ('imul', ('isub', 3, b), 8)), 24), > 'options->lower_extract_byte'), > > - (('extract_u8', a, b), > + (('extract_u8', a, 'b@32'), > ('iand', ('ushr', a, ('imul', b, 8)), 0xff), > 'options->lower_extract_byte'), > > - (('extract_i16', a, b), > + (('extract_i16', a, 'b@32'), > ('ishr', ('ishl', a, ('imul', ('isub', 1, b), 16)), 16), > 'options->lower_extract_word'), > > - (('extract_u16', a, b), > + (('extract_u16', a, 'b@32'), > ('iand', ('ushr', a, ('imul', b, 16)), 0xffff), > 'options->lower_extract_word'), > > -- > 2.5.5 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev