On 2015-12-30 13:26:48, Ilia Mirkin wrote: > On Wed, Dec 30, 2015 at 3:26 PM, Matt Turner <matts...@gmail.com> wrote: > > The OpenGL specifications for these functions say: > > > > The result will be undefined if <offset> or <bits> is negative, or if > > the sum of <offset> and <bits> is greater than the number of bits > > used to store the operand. > > > > Therefore passing bits=32, offset=0 is legal and defined in GLSL. > > > > But the earlier DX11/SM5 bfi/ibfe/ubfe opcodes are specified to accept a > > bitfield width ranging from 0-31. As such, Intel and AMD instructions > > read only the low 5 bits of the width operand, making them not compliant > > with the GLSL spec, so we have to special case the bits=32 case. > > > > Checking that offset=0 is not necessary, since for any other value, > > <offset> + <bits> will be greater than 32, which is specified as > > generating an undefined result. > > > > Fixes: > > ES31-CTS.shader_bitfield_operation.bitfieldInsert.uint_2 > > ES31-CTS.shader_bitfield_operation.bitfieldInsert.uvec4_3 > > ES31-CTS.shader_bitfield_operation.bitfieldExtract.uvec3_0 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92595 > > --- > > Yuck. Suggestions welcome. > > Can you make a piglit test? Want to see if nvidia has the same > problem. According to > http://docs.nvidia.com/cuda/parallel-thread-execution/#integer-arithmetic-instructions-bfe, > offset/bits can actually be up to 255 (although I can't fully imagine > why one might want that). However perhaps the HW differs. >
Matt, Should we move this into the driver then? -Jordan > > > > > src/glsl/builtin_functions.cpp | 6 +++++- > > src/glsl/lower_instructions.cpp | 7 +++---- > > 2 files changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp > > index 602852a..3d5de83 100644 > > --- a/src/glsl/builtin_functions.cpp > > +++ b/src/glsl/builtin_functions.cpp > > @@ -4894,7 +4894,11 @@ builtin_builder::_bitfieldExtract(const glsl_type > > *type) > > ir_variable *bits = in_var(glsl_type::int_type, "bits"); > > MAKE_SIG(type, gpu_shader5_or_es31, 3, value, offset, bits); > > > > - body.emit(ret(expr(ir_triop_bitfield_extract, value, offset, bits))); > > + ir_if *if_32 = new(mem_ctx) ir_if(greater(bits, imm(31))); > > + if_32->then_instructions.push_tail(ret(rshift(value, offset))); > > + if_32->else_instructions.push_tail( > > + ret(expr(ir_triop_bitfield_extract, value, offset, bits))); > > + body.emit(if_32); > > > > return sig; > > } > > diff --git a/src/glsl/lower_instructions.cpp > > b/src/glsl/lower_instructions.cpp > > index 845cfff..8a425a8 100644 > > --- a/src/glsl/lower_instructions.cpp > > +++ b/src/glsl/lower_instructions.cpp > > @@ -359,10 +359,9 @@ > > lower_instructions_visitor::bitfield_insert_to_bfm_bfi(ir_expression *ir) > > ir_rvalue *base_expr = ir->operands[0]; > > > > ir->operation = ir_triop_bfi; > > - ir->operands[0] = new(ir) ir_expression(ir_binop_bfm, > > - ir->type->get_base_type(), > > - ir->operands[3], > > - ir->operands[2]); > > + ir->operands[0] = lshift(rshift(new(ir) ir_constant(~0u), > > + sub(new(ir) ir_constant(32), > > ir->operands[3])), > > + ir->operands[2]); > > /* ir->operands[1] is still the value to insert. */ > > ir->operands[2] = base_expr; > > ir->operands[3] = NULL; > > -- > > 2.4.9 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
