Without this commit, copy propagation is discarded if it involves a uniform with an instruction that has 3 sources. But 3 sourced instructions can access scalar values.
For example, this is what vec4_visitor::fix_3src_operand() is already doing: if (src.file == UNIFORM && brw_is_single_value_swizzle(src.swizzle)) return src; Shader-db results (unfiltered) on NIR: total instructions in shared programs: 6259650 -> 6241985 (-0.28%) instructions in affected programs: 812755 -> 795090 (-2.17%) helped: 7930 HURT: 0 Shader-db results (unfiltered) on IR: total instructions in shared programs: 6445822 -> 6441788 (-0.06%) instructions in affected programs: 296630 -> 292596 (-1.36%) helped: 2533 HURT: 0 v2: - Updated commit message, using Matt Turner suggestions - Move the check after we've created the final value, as Jason Ekstrand suggested - Clean up the condition v3: - Move the check back to the original place, to keep things tidy, as suggested by Jason Ekstrand --- > Why did you move this up here? If we're going to have checks based on > the final value, they should probably go after we've constructed the > entire final value. Does that seem reasonable? So I'd put this back > and just put the UNIFORM 3-src check below it. Otherwise, we're > breaking up the "build the final value" again. The problem is that at the end of try_copy_propagate we are not only creating the entire final value, but also modifying some of the instruction properties, based on the fact that it is safe at that point. > If you don't like that, then I'd rather just go with the first patch. This commit is basically the original one, but with some cleanings, including Matt's suggestion on the if, plus calling compose_swizzle just once. I personally find more readable this way, sorry for the noise of asking another patch review. src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp index d3f0ddd..158b25d 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp @@ -325,7 +325,11 @@ try_copy_propagate(const struct brw_device_info *devinfo, inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_WRITE) return false; - if (inst->is_3src() && value.file == UNIFORM) + unsigned composed_swizzle = brw_compose_swizzle(inst->src[arg].swizzle, + value.swizzle); + if (inst->is_3src() && + value.file == UNIFORM && + !composed_swizzle) return false; if (inst->is_send_from_grf()) @@ -380,8 +384,7 @@ try_copy_propagate(const struct brw_device_info *devinfo, if (inst->src[arg].negate) value.negate = !value.negate; - value.swizzle = brw_compose_swizzle(inst->src[arg].swizzle, - value.swizzle); + value.swizzle = composed_swizzle; if (has_source_modifiers && value.type != inst->src[arg].type) { assert(can_change_source_types(inst)); -- 2.1.4 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev