diff options
| author | Ian Romanick <ian.d.romanick@intel.com> | 2024-06-25 13:30:53 -0700 |
|---|---|---|
| committer | Eric Engestrom <eric@engestrom.ch> | 2024-08-14 11:52:16 +0200 |
| commit | fef088fd5de61719ffa117bd0b569dbbfd247720 (patch) | |
| tree | b7bfe31505c77859a5d9b13d5a2d9e039ebaede6 /src/intel | |
| parent | 6f625b1b9510d7412c988b1048119945432b4849 (diff) | |
intel/elk: Don't propagate saturate to an instruction that writes flags
There are two problems.
1. This is not NaN safe. 'add.le.sat dst F, Inf F, -Inf F' has a
different result than 'add dst F, Inf F, -Inf F; cmp.le null, dst F, 0F'.
2. Ignoring the first problem, this only produces the desired flags
for LE and G. All other cases can produce the wrong result.
shader-db:
All Intel platforms had similar results. (Broadwell shown)
total instructions in shared programs: 18282314 -> 18282316 (<.01%)
instructions in affected programs: 78 -> 80 (2.56%)
helped: 0
HURT: 2
total cycles in shared programs: 952924234 -> 952924252 (<.01%)
cycles in affected programs: 584 -> 602 (3.08%)
helped: 0
HURT: 2
Fixes: e6022281f27 ("intel/elk: Rename files to use elk prefix")
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29774>
(cherry picked from commit 9125b7c1b41c75a4f39ce2949f74530ae7f1fde0)
Diffstat (limited to 'src/intel')
| -rw-r--r-- | src/intel/compiler/elk/elk_fs_saturate_propagation.cpp | 15 |
1 files changed, 13 insertions, 2 deletions
diff --git a/src/intel/compiler/elk/elk_fs_saturate_propagation.cpp b/src/intel/compiler/elk/elk_fs_saturate_propagation.cpp index 73910dd1f63..5a3520e1042 100644 --- a/src/intel/compiler/elk/elk_fs_saturate_propagation.cpp +++ b/src/intel/compiler/elk/elk_fs_saturate_propagation.cpp @@ -45,7 +45,8 @@ using namespace elk; */ static bool -opt_saturate_propagation_local(const fs_live_variables &live, elk_bblock_t *block) +opt_saturate_propagation_local(const intel_device_info *devinfo, + const fs_live_variables &live, elk_bblock_t *block) { bool progress = false; int ip = block->end_ip + 1; @@ -74,6 +75,16 @@ opt_saturate_propagation_local(const fs_live_variables &live, elk_bblock_t *bloc !scan_inst->can_change_types())) break; + /* min and max pseudo ops modify the flags on Gfx4 and Gfx5, but + * it's not based on the result of the operation. This is the one + * case where it is always safe to propagate a saturate to an + * instruction that writes the flags. + */ + if (scan_inst->flags_written(devinfo) != 0 && + scan_inst->opcode != ELK_OPCODE_SEL) { + break; + } + if (scan_inst->saturate) { inst->saturate = false; progress = true; @@ -156,7 +167,7 @@ elk_fs_visitor::opt_saturate_propagation() bool progress = false; foreach_block (block, cfg) { - progress = opt_saturate_propagation_local(live, block) || progress; + progress = opt_saturate_propagation_local(devinfo, live, block) || progress; } /* Live intervals are still valid. */ |