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 <imir...@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 <curroje...@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 <v...@freedesktop.org>
>> Cc: mesa-sta...@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
