<div dir="auto">I'm sure radeonsi will handle UCMP with modifiers too. (which is basically gallivm)<div dir="auto"> </div><div dir="auto">Marek</div></div><div class="gmail_extra"> <div class="gmail_quote">On Mar 6, 2017 1:06 PM, "Roland Scheidegger" <<a href="mailto:sroland@vmware.com">sroland@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Am 04.03.2017 um 20:45 schrieb 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? float modifiers used with UCMP are actually just fine in theory - the UCMP sources are considered floats for the purposes of modifiers (this is similar to mov which also has untyped sources, except that 1 of the arguments of ucmp indeed is a uint, which cannot have modifiers). (tgsi_opcode_infer_type() says it's untyped, but tgsi_opcode_infer_src_type() says it's float though obviously this doesn't apply to the condition argument.) Therefore, not handling float modifiers with ucmp src args is a bug of the tgsi executor (I'm quite sure llvmpipe will handle it right). Looks like all source arguments must be the same type there... Seems annoying to fix actually... Albeit the gallium docs don't really mention this (this is the same behavior as dx10 movc). I don't know though if other drivers will handle it correctly, but if they do it might be a better idea to just fix tgsi_exec somehow. (I'm not sure if the opposite could happen, that is int modifiers mistakenly going into a ucmp, or if this could be a problem with other instructions.) Roland > -ilia > > > On Sat, Mar 4, 2017 at 2:24 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> 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 <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> 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: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D99817&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=VpboBTuXbZaDL1-iEDED9JFLZTZFsPVqjTSbTuGSuT8&e=" rel="noreferrer" target="_blank">https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D99817&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=VpboBTuXbZaDL1-iEDED9JFLZTZFsPVqjTSbTuGSuT8&e=</a> >>> Tested-by: Vinson Lee <<a href="mailto:vlee@freedesktop.org">vlee@freedesktop.org</a>> >>> Cc: <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a> >>> --- >>> 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 >>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >>> <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=zkVQuaiGqon4ebXPnKm96V6UnqAVXfGc_ky9Lm_FMKY&e=" rel="noreferrer" target="_blank">https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=zkVQuaiGqon4ebXPnKm96V6UnqAVXfGc_ky9Lm_FMKY&e=</a> > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=zkVQuaiGqon4ebXPnKm96V6UnqAVXfGc_ky9Lm_FMKY&e=" rel="noreferrer" target="_blank">https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=ak9IRpzPmkscMdh984JOu0Zn2I-efj9Iy3K-Iw9vv04&s=zkVQuaiGqon4ebXPnKm96V6UnqAVXfGc_ky9Lm_FMKY&e=</a> > _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>