<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Oct 11, 2018 at 1:47 PM Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 10/11/2018 11:36 AM, Jason Ekstrand wrote:<br>
> On Thu, Oct 11, 2018 at 1:23 PM Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a><br>
> <mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote:<br>
> <br>
>     On 10/11/2018 11:21 AM, Ian Romanick wrote:<br>
>     > On 10/11/2018 08:38 AM, Jason Ekstrand wrote:<br>
>     >> ---<br>
>     >>  src/intel/compiler/brw_vec4_nir.cpp | 7 ++++++-<br>
>     >>  1 file changed, 6 insertions(+), 1 deletion(-)<br>
>     >><br>
>     >> diff --git a/src/intel/compiler/brw_vec4_nir.cpp<br>
>     b/src/intel/compiler/brw_vec4_nir.cpp<br>
>     >> index eaf1754b006..5ccfd1f8940 100644<br>
>     >> --- a/src/intel/compiler/brw_vec4_nir.cpp<br>
>     >> +++ b/src/intel/compiler/brw_vec4_nir.cpp<br>
>     >> @@ -1586,7 +1586,12 @@ vec4_visitor::nir_emit_alu(nir_alu_instr<br>
>     *instr)<br>
>     >> <br>
>     >>     case nir_op_b2i:<br>
>     >>     case nir_op_b2f:<br>
>     >> -      emit(MOV(dst, negate(op[0])));<br>
>     >> +      if (nir_dest_bit_size(instr->dest.dest) > 32) {<br>
>     >> +         assert(dst.type == BRW_REGISTER_TYPE_DF);<br>
>     >> +         emit_conversion_to_double(dst, negate(op[0]), false);<br>
>     >> +      } else {<br>
>     >> +         emit(MOV(dst, negate(op[0])));<br>
>     >> +      }<br>
>     ><br>
>     > I wrote almost the same patch, but I split nir_op_b2i and nir_op_b2f<br>
>     > handling and omitted the assert.  I was never able to get the<br>
>     > then-clause to trigger.  Were you able to get that to happen?<br>
> <br>
>     I guess the next patch causes it?  Assuming that's the case, you should<br>
>     add that to the commit message for this patch.<br>
> <br>
> <br>
> Correct.  Sure, I can add that to the commit message.  I was going to CC<br>
> it to stable because I don't really know if it's possible to trigger or<br>
> not.  It definitely does trigger with the previous patch but I can't<br>
> prove that it doesn't trigger without it.  Thoughts?<br>
<br>
Unless there's some way to do it from SPIR-V, I don't think it's<br>
possible to trigger either the FS case or the vec4 case without at least<br>
a68dd47b911.  Assuming that's true, I don't think any of these need to<br>
go to stable.</blockquote><div><br></div><div>Surprisingly, SPIR-V doesn't have cast opcodes to/from bool.  I guess they considered Sel and != to be sufficient.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">GLSL IR doesn't have a bool-to-double opcode, so it will<br>
always generate something like double(float(boolean_value)).  It might<br>
be worth adding that to this commit message too.  The commit message for<br>
patch 1 already implies this.<br></blockquote><div><br></div><div>Ok, I'll add a note.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
With whatever commit message changes you deem appropriate, this series is<br>
<br>
Reviewed-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><br></blockquote><div><br></div><div>Thanks!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>     >>        break;<br>
>     >> <br>
>     >>     case nir_op_f2b:<br>
>     ><br>
>     > _______________________________________________<br>
>     > mesa-dev mailing list<br>
>     > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>><br>
>     > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>