Module: Mesa Branch: main Commit: 782de1932c84a27e25e1fcf593228f7bcaf248e4 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=782de1932c84a27e25e1fcf593228f7bcaf248e4
Author: Ian Romanick <[email protected]> Date: Thu Mar 23 14:19:29 2023 -0700 intel/fs: Don't copy propagate from saturate to sel There are already NIR algebraic optimizations (see also ac6646129f7 ("nir: Move fsat outside of fmin/fmax if second arg is 0 to 1.") that will try to remove the saturate from things like fmax(0.5, fsat(x)) This basically reverts 40aeb558ce8 ("i965/fs: Allow propagation of instructions with saturate flag to sel"). That commit message had no shader-db information, so it's unclear whether this actually helped anything ever. No shader-db changes on any Intel platform. One shader in Far Cry New Dawn was affected. Cycles in all programs: 10933090738 -> 10933090736 (-0.0%) Cycles helped: 1 Reviewed-by: Matt Turner <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22169> --- src/intel/compiler/brw_fs_copy_propagation.cpp | 24 +----------------------- src/intel/compiler/test_fs_copy_propagation.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 29 deletions(-) diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp index 60603ba4557..05556943dfc 100644 --- a/src/intel/compiler/brw_fs_copy_propagation.cpp +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp @@ -51,7 +51,6 @@ struct acp_entry : public exec_node { unsigned size_written; unsigned size_read; enum opcode opcode; - bool saturate; bool is_partial_write; bool force_writemask_all; }; @@ -722,22 +721,6 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry) return false; } - if (entry->saturate) { - switch(inst->opcode) { - case BRW_OPCODE_SEL: - if ((inst->conditional_mod != BRW_CONDITIONAL_GE && - inst->conditional_mod != BRW_CONDITIONAL_L) || - inst->src[1].file != IMM || - inst->src[1].f < 0.0 || - inst->src[1].f > 1.0) { - return false; - } - break; - default: - return false; - } - } - /* Save the offset of inst->src[arg] relative to entry->dst for it to be * applied later. */ @@ -772,9 +755,6 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry) inst->src[arg].stride *= entry->src.stride; } - /* Compose any saturate modifiers. */ - inst->saturate = inst->saturate || entry->saturate; - /* Compute the first component of the copy that the instruction is * reading, and the base byte offset within that component. */ @@ -821,8 +801,6 @@ fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry) return false; if (type_sz(entry->src.type) > 4) return false; - if (entry->saturate) - return false; for (int i = inst->sources - 1; i >= 0; i--) { if (inst->src[i].file != VGRF) @@ -1105,6 +1083,7 @@ can_propagate_from(fs_inst *inst) (inst->src[0].file == FIXED_GRF && inst->src[0].is_contiguous())) && inst->src[0].type == inst->dst.type && + !inst->saturate && /* Subset of !is_partial_write() conditions. */ !((inst->predicate && inst->opcode != BRW_OPCODE_SEL) || !inst->dst.is_contiguous())) || @@ -1168,7 +1147,6 @@ fs_visitor::opt_copy_propagation_local(void *copy_prop_ctx, bblock_t *block, for (unsigned i = 0; i < inst->sources; i++) entry->size_read += inst->size_read(i); entry->opcode = inst->opcode; - entry->saturate = inst->saturate; entry->is_partial_write = inst->is_partial_write(); entry->force_writemask_all = inst->force_writemask_all; acp[entry->dst.nr % ACP_HASH_SIZE].push_tail(entry); diff --git a/src/intel/compiler/test_fs_copy_propagation.cpp b/src/intel/compiler/test_fs_copy_propagation.cpp index 2de13476032..c66f64fd139 100644 --- a/src/intel/compiler/test_fs_copy_propagation.cpp +++ b/src/intel/compiler/test_fs_copy_propagation.cpp @@ -164,12 +164,12 @@ TEST_F(copy_propagation_test, maxmax_sat_imm) bool expected_result; } test[] = { /* conditional mod, imm, expected_result */ - { BRW_CONDITIONAL_GE , 0.1f, true }, - { BRW_CONDITIONAL_L , 0.1f, true }, - { BRW_CONDITIONAL_GE , 0.5f, true }, - { BRW_CONDITIONAL_L , 0.5f, true }, - { BRW_CONDITIONAL_GE , 0.9f, true }, - { BRW_CONDITIONAL_L , 0.9f, true }, + { BRW_CONDITIONAL_GE , 0.1f, false }, + { BRW_CONDITIONAL_L , 0.1f, false }, + { BRW_CONDITIONAL_GE , 0.5f, false }, + { BRW_CONDITIONAL_L , 0.5f, false }, + { BRW_CONDITIONAL_GE , 0.9f, false }, + { BRW_CONDITIONAL_L , 0.9f, false }, { BRW_CONDITIONAL_GE , -1.5f, false }, { BRW_CONDITIONAL_L , -1.5f, false }, { BRW_CONDITIONAL_GE , 1.5f, false },
