[Mesa-dev] [PATCH 2/3] intel/vec4: Fix nir_op_b2[fi] with 64-bit result

[Jason Ekstrand]

On Thu, Oct 11, 2018 at 1:47 PM Ian Romanick <idr at freedesktop.org> wrote:

> On 10/11/2018 11:36 AM, Jason Ekstrand wrote:
> > On Thu, Oct 11, 2018 at 1:23 PM Ian Romanick <idr at freedesktop.org
> > <mailto:idr at freedesktop.org>> wrote:
> >
> >     On 10/11/2018 11:21 AM, Ian Romanick wrote:
> >     > On 10/11/2018 08:38 AM, Jason Ekstrand wrote:
> >     >> ---
> >     >>  src/intel/compiler/brw_vec4_nir.cpp | 7 ++++++-
> >     >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >     >>
> >     >> diff --git a/src/intel/compiler/brw_vec4_nir.cpp
> >     b/src/intel/compiler/brw_vec4_nir.cpp
> >     >> index eaf1754b006..5ccfd1f8940 100644
> >     >> --- a/src/intel/compiler/brw_vec4_nir.cpp
> >     >> +++ b/src/intel/compiler/brw_vec4_nir.cpp
> >     >> @@ -1586,7 +1586,12 @@ vec4_visitor::nir_emit_alu(nir_alu_instr
> >     *instr)
> >     >>
> >     >>     case nir_op_b2i:
> >     >>     case nir_op_b2f:
> >     >> -      emit(MOV(dst, negate(op[0])));
> >     >> +      if (nir_dest_bit_size(instr->dest.dest) > 32) {
> >     >> +         assert(dst.type == BRW_REGISTER_TYPE_DF);
> >     >> +         emit_conversion_to_double(dst, negate(op[0]), false);
> >     >> +      } else {
> >     >> +         emit(MOV(dst, negate(op[0])));
> >     >> +      }
> >     >
> >     > I wrote almost the same patch, but I split nir_op_b2i and
> nir_op_b2f
> >     > handling and omitted the assert.  I was never able to get the
> >     > then-clause to trigger.  Were you able to get that to happen?
> >
> >     I guess the next patch causes it?  Assuming that's the case, you
> should
> >     add that to the commit message for this patch.
> >
> >
> > Correct.  Sure, I can add that to the commit message.  I was going to CC
> > it to stable because I don't really know if it's possible to trigger or
> > not.  It definitely does trigger with the previous patch but I can't
> > prove that it doesn't trigger without it.  Thoughts?
>
> Unless there's some way to do it from SPIR-V, I don't think it's
> possible to trigger either the FS case or the vec4 case without at least
> a68dd47b911.  Assuming that's true, I don't think any of these need to
> go to stable.


Surprisingly, SPIR-V doesn't have cast opcodes to/from bool.  I guess they
considered Sel and != to be sufficient.


> GLSL IR doesn't have a bool-to-double opcode, so it will
> always generate something like double(float(boolean_value)).  It might
> be worth adding that to this commit message too.  The commit message for
> patch 1 already implies this.
>

Ok, I'll add a note.


> With whatever commit message changes you deem appropriate, this series is
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>

Thanks!


> >     >>        break;
> >     >>
> >     >>     case nir_op_f2b:
> >     >
> >     > _______________________________________________
> >     > mesa-dev mailing list
> >     > mesa-dev at lists.freedesktop.org <mailto:
> mesa-dev at lists.freedesktop.org>
> >     > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181011/b7d84167/attachment.html>