[Mesa-stable] [Mesa-dev] [PATCH] st/glsl_to_tgsi: Translate float ir_unop_neg into float MOV with modifier.

[Ilia Mirkin]

Also, how is this happening in the first place? For example, we have:

   case ir_unop_bitcast_f2i:
   case ir_unop_bitcast_f2u:
      /* Make sure we don't propagate the negate modifier to integer opcodes. */
      if (op[0].negate || op[0].abs)
         emit_asm(ir, TGSI_OPCODE_MOV, result_dst, op[0]);
      else
         result_src = op[0];

Oh, but it's going directly into a ir_triop_csel, which is missing
this logic. It should be added there instead, IMHO. OTOH, the same
issue will hit in emit_block_mov() if you do. Would love to hear some
other opinions... Marek, Brian, Roland?

  -ilia


On Sat, Mar 4, 2017 at 2:24 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Hmmm... I wonder if this should only be done for the native_integers
> case. I'm concerned that this will cause perf regressions on weaker hw
> like nv30 and r300, as the neg will no longer be inserted as a
> modifier into the next instruction. Any opinion on this?
>
> On Sat, Mar 4, 2017 at 2:16 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Otherwise result_src may be provided to an integer instruction whose
>> negate modifier has different semantics.  Example is UCMP as in the
>> bug linked below, where an unrelated change in the GLSL built-in
>> lowering code for atan2 (e9ffd12827ac11a2d2002a42fa8eb1df847153ba)
>> caused the generation of floating-point ir_unop_neg instructions
>> followed by ir_triop_csel, which is lowered into UCMP on back-ends
>> with native integer support.
>>
>> Fixes a regression of piglit glsl-fs-tan-1 on softpipe introduced by
>> the above-mentioned glsl front-end commit.  Even though the commit
>> that triggered the regression doesn't seem to have made it to any
>> stable branches yet, this seems worth back-porting since I don't see
>> any reason why the bug couldn't have been reproduced before that
>> point.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99817
>> Tested-by: Vinson Lee <vlee at freedesktop.org>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index af41bdb..6bf3c89 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -1633,7 +1633,7 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* ir, st_src_reg *op)
>>           emit_asm(ir, TGSI_OPCODE_DNEG, result_dst, op[0]);
>>        else {
>>           op[0].negate = ~op[0].negate;
>> -         result_src = op[0];
>> +         emit_asm(ir, TGSI_OPCODE_MOV, result_dst, op[0]);
>>        }
>>        break;
>>     case ir_unop_subroutine_to_int:
>> --
>> 2.10.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev