On 07/05/2018 06:22 PM, Caio Marcelo de Oliveira Filho wrote: > Reviewed-by: Caio Marcelo de Oliveira Filho <[email protected]> > > Tested the patch with the shaders mentioned. > > Do you think would be worth to have a pass at NIR level to perform > these transformations?
Possibly. In many cases, it is better to keep a negation or an abs on the inside of an expression instead of the outside (e.g., sign(-x) vs -sign(x)). We can almost always propagate an outer negation to the user, but there are some cases, such as when the user is in a different basic block, where it cannot be done. In those cases, the outer unary op results in an extra move. It is difficult to move unary ops inside because it can interfere with other optimizations. When I have tried this in the past, it has generally been an even split between helps / hurts. > Thanks, > Caio > > > On Wed, Jun 27, 2018 at 07:37:57AM -0700, Ian Romanick wrote: >> From: Ian Romanick <[email protected]> >> >> Fixes new piglit tests: >> >> - glsl-1.10/execution/fs-sign-neg-abs.shader_test >> - glsl-1.10/execution/fs-sign-sat-neg-abs.shader_test >> - glsl-1.10/execution/vs-sign-neg-abs.shader_test >> - glsl-1.10/execution/vs-sign-sat-neg-abs.shader_test >> >> Signed-off-by: Ian Romanick <[email protected]> >> Cc: [email protected] >> --- >> src/intel/compiler/brw_fs_nir.cpp | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/src/intel/compiler/brw_fs_nir.cpp >> b/src/intel/compiler/brw_fs_nir.cpp >> index 0abb4798e70..2ad7a2d0dfc 100644 >> --- a/src/intel/compiler/brw_fs_nir.cpp >> +++ b/src/intel/compiler/brw_fs_nir.cpp >> @@ -807,11 +807,20 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, >> nir_alu_instr *instr) >> >> case nir_op_fsign: { >> if (op[0].abs) { >> - /* Straightforward since the source can be assumed to be >> - * non-negative. >> + /* Straightforward since the source can be assumed to be either >> + * strictly >= 0 or strictly <= 0 depending on the setting of the >> + * negate flag. >> */ >> set_condmod(BRW_CONDITIONAL_NZ, bld.MOV(result, op[0])); >> - set_predicate(BRW_PREDICATE_NORMAL, bld.MOV(result, >> brw_imm_f(1.0f))); >> + >> + inst = (op[0].negate) >> + ? bld.MOV(result, brw_imm_f(-1.0f)) >> + : bld.MOV(result, brw_imm_f(1.0f)); >> + >> + set_predicate(BRW_PREDICATE_NORMAL, inst); >> + >> + if (instr->dest.saturate) >> + inst->saturate = true; >> >> } else if (type_sz(op[0].type) < 8) { >> /* AND(val, 0x80000000) gives the sign bit. >> -- >> 2.14.4 >> >> _______________________________________________ >> mesa-dev mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
