[Mesa-dev] [PATCH 2/2] glsl: Handle bits=32 case in bitfieldInsert/bitfieldExtract.
Jordan Justen
jordan.l.justen at intel.com
Sat Jan 2 09:18:25 PST 2016
On 2015-12-30 13:26:48, Ilia Mirkin wrote:
> On Wed, Dec 30, 2015 at 3:26 PM, Matt Turner <mattst88 at 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list