If a source operand in a MOV has source modifiers, then we cannot copy-propagate it from the parent instruction and remove the MOV. ---
I noticed this while debugging some regressions introduced with the fp64 code. Basically, I had code similar to this: vec4 ssa1 = intrincisc1 (...) (...) vec2 ssa2 = vec2 ssa1 -ssa1.y vec4 ssa3 = intrinsic2 (ssa2) (...) that would be turned into this by copy propagation: vec4 ssa1 = some intrincisc vec4 ssa2 = some intrinsic (ssa1) which is obviously not correct. This was happening because is_swizzleless_move checked that the MOV/vecN operation that generates the value we want to copy-propagate does not incorporate swizzling, but it ignored the case where it also added source modifiers, in which case we can't copy-propagate either. Of course, now that we have made vecN operations unsigned again, that example can't happen because lower_to_source_mods won't produce things like that, but I figured that the patch would still make sense, since it fixes a case where copy-propagation won't work as intended, even if we are not currently triggering it (at least not with vecN operations). src/glsl/nir/nir_opt_copy_propagate.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/glsl/nir/nir_opt_copy_propagate.c b/src/glsl/nir/nir_opt_copy_propagate.c index 7d8bdd7..7caa4b7 100644 --- a/src/glsl/nir/nir_opt_copy_propagate.c +++ b/src/glsl/nir/nir_opt_copy_propagate.c @@ -65,9 +65,12 @@ static bool is_vec(nir_alu_instr *instr) } static bool -is_swizzleless_move(nir_alu_instr *instr) +is_simple_move(nir_alu_instr *instr) { if (is_move(instr)) { + if (instr->src[0].negate || instr->src[0].abs) + return false; + for (unsigned i = 0; i < 4; i++) { if (!((instr->dest.write_mask >> i) & 1)) break; @@ -81,6 +84,9 @@ is_swizzleless_move(nir_alu_instr *instr) if (instr->src[i].swizzle[0] != i) return false; + if (instr->src[i].negate || instr->src[i].abs) + return false; + if (def == NULL) { def = instr->src[i].src.ssa; } else if (instr->src[i].src.ssa != def) { @@ -107,7 +113,7 @@ copy_prop_src(nir_src *src, nir_instr *parent_instr, nir_if *parent_if) return false; nir_alu_instr *alu_instr = nir_instr_as_alu(src_instr); - if (!is_swizzleless_move(alu_instr)) + if (!is_simple_move(alu_instr)) return false; /* Don't let copy propagation land us with a phi that has more -- 1.9.1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev