On 01/11/2016 04:14 PM, Matt Turner wrote: > On Mon, Jan 11, 2016 at 4:01 PM, Ian Romanick <[email protected]> wrote: >> On 01/11/2016 02:48 PM, Matt Turner wrote: >>> Intel/AMD's hardware instructions do not handle arguments of 32. >>> Constant evaluation should not produce a result different from the >>> hardware instruction. >>> --- >>> src/glsl/nir/nir_opcodes.py | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/glsl/nir/nir_opcodes.py b/src/glsl/nir/nir_opcodes.py >>> index 9e92fab..ccf9498 100644 >>> --- a/src/glsl/nir/nir_opcodes.py >>> +++ b/src/glsl/nir/nir_opcodes.py >>> @@ -511,12 +511,15 @@ binop("fpow", tfloat, "", "powf(src0, src1)") >>> binop_horiz("pack_half_2x16_split", 1, tuint, 1, tfloat, 1, tfloat, >>> "pack_half_1x16(src0.x) | (pack_half_1x16(src1.x) << 16)") >>> >>> +# bfm implements the behavior of the first operation of the SM5 "bfi" >>> assembly >>> +# and that of the "bfi1" i965 instruction. That is, it has undefined >>> behavior >>> +# if either of its arguments are 32. >>> binop_convert("bfm", tuint, tint, "", """ >>> int bits = src0, offset = src1; >>> -if (offset < 0 || bits < 0 || offset + bits > 32) >>> - dst = 0; /* undefined per the spec */ >>> +if (offset < 0 || bits < 0 || offset > 31 || bits > 31 || offset + bits > >>> 32) >>> + dst = 0; /* undefined */ >>> else >>> - dst = ((1ul << bits) - 1) << offset; >>> + dst = ((1u << bits) - 1) << offset; >> >> Was s/1ul/1u/ intentional? > > Yes -- previously we wanted to handle the "1 << 32" case so we needed > to use 1ul (actually, 1ull I suppose?) but now we're okay with that > being undefined so by changing s/1ul/1ull/ we save a little bit by not > needing a 64-bit operation.
That makes sense. Maybe mention that in the commit message? Either way, this patch is Reviewed-by: Ian Romanick <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
