From b2d7a823be83ed300462bd1b1ca329373da201bc Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Tue, 29 Oct 2024 14:41:19 -0700 Subject: [PATCH] brw/lower: Don't emit spurious moves to or from NULL register Previously an instruction like cmp.l.f0.0(16) null:F, v359:F, 0f would get lowered to undef(16) v13703:UD cmp.l.f0.0(16) v13703:F, v359:F, 0f mov(16) null:UD, v13703:UD After copy propagation and dead-code elimination are run again, the original CMP gets turned back into its original form! Some cases can also emit MOVs from the original NULL register. It should be possible to not do any lowering here, but there are some interactions with source lowering passes for things like cmp.l.f0.0(16) null:HF, g89.1<16,16,1>:HF, 0hf What inspired this was... diff'ing step-by-step dumps from INTEL_DEBUG=optimizer had a lot of useless changes due to these MOVs and undefs. It was very annoying. This low-effort change gets the majority of the possible benefit. No shader-db or fossil-db changes on any Intel platform. Reviewed-by: Kenneth Graunke Part-of: --- src/intel/compiler/brw_fs_lower_regioning.cpp | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/intel/compiler/brw_fs_lower_regioning.cpp b/src/intel/compiler/brw_fs_lower_regioning.cpp index 2364ed1c3f9..86c72bb53be 100644 --- a/src/intel/compiler/brw_fs_lower_regioning.cpp +++ b/src/intel/compiler/brw_fs_lower_regioning.cpp @@ -320,7 +320,7 @@ namespace { return (has_dst_aligned_region_restriction(devinfo, inst) && !is_uniform(inst->src[i]) && - (byte_stride(inst->src[i]) != byte_stride(inst->dst) || + (byte_stride(inst->src[i]) != required_src_byte_stride(devinfo, inst, i) || src_byte_offset != dst_byte_offset)) || (has_subdword_integer_region_restriction(devinfo, inst) && (byte_stride(inst->src[i]) != required_src_byte_stride(devinfo, inst, i) || @@ -628,35 +628,39 @@ namespace { ibld.UNDEF(tmp); tmp = horiz_stride(tmp, stride); - /* Emit a series of 32-bit integer copies from the temporary into the - * original destination. - */ - const brw_reg_type raw_type = brw_int_type(MIN2(brw_type_size_bytes(tmp.type), 4), - false); - const unsigned n = brw_type_size_bytes(tmp.type) / brw_type_size_bytes(raw_type); - - if (inst->predicate && inst->opcode != BRW_OPCODE_SEL) { - /* Note that in general we cannot simply predicate the copies on the - * same flag register as the original instruction, since it may have - * been overwritten by the instruction itself. Instead initialize - * the temporary with the previous contents of the destination - * register. + if (!inst->dst.is_null()) { + /* Emit a series of 32-bit integer copies from the temporary into the + * original destination. */ + const brw_reg_type raw_type = + brw_int_type(MIN2(brw_type_size_bytes(tmp.type), 4), false); + + const unsigned n = + brw_type_size_bytes(tmp.type) / brw_type_size_bytes(raw_type); + + if (inst->predicate && inst->opcode != BRW_OPCODE_SEL) { + /* Note that in general we cannot simply predicate the copies on + * the same flag register as the original instruction, since it + * may have been overwritten by the instruction itself. Instead + * initialize the temporary with the previous contents of the + * destination register. + */ + for (unsigned j = 0; j < n; j++) + ibld.MOV(subscript(tmp, raw_type, j), + subscript(inst->dst, raw_type, j)); + } + for (unsigned j = 0; j < n; j++) - ibld.MOV(subscript(tmp, raw_type, j), - subscript(inst->dst, raw_type, j)); + ibld.at(block, inst->next).MOV(subscript(inst->dst, raw_type, j), + subscript(tmp, raw_type, j)); + + /* If the destination was an accumulator, after lowering it will be a + * GRF. Clear writes_accumulator for the instruction. + */ + if (inst->dst.is_accumulator()) + inst->writes_accumulator = false; } - for (unsigned j = 0; j < n; j++) - ibld.at(block, inst->next).MOV(subscript(inst->dst, raw_type, j), - subscript(tmp, raw_type, j)); - - /* If the destination was an accumulator, after lowering it will be a - * GRF. Clear writes_accumulator for the instruction. - */ - if (inst->dst.is_accumulator()) - inst->writes_accumulator = false; - /* Point the original instruction at the temporary, making sure to keep * any destination modifiers in the instruction. */