From 3d23496fd99f854c0c71b442257971f7e6dd8f98 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Thu, 27 Feb 2025 15:10:25 -0800 Subject: [PATCH] brw/copy: Copy prop -X into Y&1 This commit prevents code quality regressions in the next commit. Without this, some fragment shaders in Batman: Arkham Origins have code like: shr(8) g51<1>UW g1.28<1,8,0>UB 0x76543210V ... and(8) g52<1>UD ~g51<8,8,1>UW 0x0001UW ... add(8) g56<1>D -g52<8,8,1>D 1D transformed to shr(8) g51<1>UW g1.28<1,8,0>UB 0x76543210V ... and(8) g52<1>UD ~g51<8,8,1>UW 0x0001UW ... mov(8) g56<1>D -g52<8,8,1>D ... and(8) g57<1>UD ~g56<8,8,1>D 0x00000001UD Propagating through the negation allows the added MOV to be deleted. shader-db: All Intel platforms had simlar results. (Lunar Lake shown) total instructions in shared programs: 16968020 -> 16968019 (<.01%) instructions in affected programs: 281 -> 280 (-0.36%) helped: 1 / HURT: 0 total cycles in shared programs: 914598850 -> 914598832 (<.01%) cycles in affected programs: 5398 -> 5380 (-0.33%) helped: 1 / HURT: 0 A single Blender vertex shader was affected. fossil-db: Lunar Lake, Tiger Lake, Ice Lake, and Skylake had similar results. (Lunar Lake shown) Totals: Instrs: 209894650 -> 209894651 (+0.00%) Cycle count: 30545958586 -> 30545952860 (-0.00%) Totals from 2 (0.00% of 706657) affected shaders: Instrs: 3582 -> 3583 (+0.03%) Cycle count: 1875100 -> 1869374 (-0.31%) Meteor Lake and DG2 had similar results. (Meteor Lake shown) Totals: Subgroup size: 9906400 -> 9906416 (+0.00%) Totals from 2 (0.00% of 805770) affected shaders: Subgroup size: 16 -> 32 (+100.00%) Two compute shaders in Hogwarts Legacy were affected. Reviewed-by: Matt Turner Part-of: --- src/intel/compiler/brw_opt_copy_propagation.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/intel/compiler/brw_opt_copy_propagation.cpp b/src/intel/compiler/brw_opt_copy_propagation.cpp index 8871dc61d1d..88f09b7ebd1 100644 --- a/src/intel/compiler/brw_opt_copy_propagation.cpp +++ b/src/intel/compiler/brw_opt_copy_propagation.cpp @@ -862,7 +862,11 @@ try_copy_propagate(brw_shader &s, brw_inst *inst, return false; if (is_logic_op(inst->opcode)) { - return false; + /* For any value of X, X & 1 = -X & 1. In this case, source modifiers + * from entry will not be applied to inst (far below). + */ + if (inst->opcode != BRW_OPCODE_AND || !inst->src[1 - arg].is_one()) + return false; } else if (entry->dst.type != inst->src[arg].type && !inst->can_change_types()) { /* Since semantics of source modifiers are type-dependent we need to @@ -921,7 +925,7 @@ try_copy_propagate(brw_shader &s, brw_inst *inst, inst->src[arg] = byte_offset(inst->src[arg], component * entry_stride * brw_type_size_bytes(entry->src.type) + suboffset); - if (has_source_modifiers) { + if (has_source_modifiers && !is_logic_op(inst->opcode)) { if (entry->dst.type != inst->src[arg].type) { /* We are propagating source modifiers from a MOV with a different * type. If we got here, then we can just change the source and @@ -1599,7 +1603,11 @@ try_copy_propagate_def(brw_shader &s, } if (is_logic_op(inst->opcode)) { - return false; + /* For any value of X, X & 1 = -X & 1. In this case, source modifiers + * from entry will not be applied to inst (far below). + */ + if (inst->opcode != BRW_OPCODE_AND || !inst->src[1 - arg].is_one()) + return false; } else if (def->dst.type != inst->src[arg].type && !inst->can_change_types()) { /* Since semantics of source modifiers are type-dependent we need to @@ -1790,7 +1798,7 @@ try_copy_propagate_def(brw_shader &s, inst->exec_size = def->exec_size; } - if (has_source_modifiers) { + if (has_source_modifiers && !is_logic_op(inst->opcode)) { if (def->dst.type != inst->src[arg].type) { /* We are propagating source modifiers from a MOV with a different * type. If we got here, then we can just change the source and