Matt Turner <matts...@gmail.com> writes: > Previously we'd ignore the sources of instructions with non-GRF > destinations when calculating calculating the dead channels. This would > lead to us incorrectly removing the first instruction in this sequence: > > mov vgrf11, ... > cmp.ne.f0 null, vgrf11, 1.0 > mov vgrf11, ... > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76616 > --- > src/mesa/drivers/dri/i965/brw_vec4.cpp | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp > b/src/mesa/drivers/dri/i965/brw_vec4.cpp > index 32a3892..4d8740a 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp > @@ -324,6 +324,9 @@ src_reg::equals(src_reg *r) > static bool > try_eliminate_instruction(vec4_instruction *inst, int new_writemask) > { > + if (inst->has_side_effects()) > + return false; > + > if (new_writemask == 0) { > /* Don't dead code eliminate instructions that write to the > * accumulator as a side-effect. Instead just set the destination > @@ -383,9 +386,6 @@ vec4_visitor::dead_code_eliminate() > > seen_control_flow = inst->is_control_flow() || seen_control_flow; > > - if (inst->has_side_effects()) > - continue; > - > bool inst_writes_flag = false; > if (inst->dst.file != GRF) { > if (inst->dst.is_null() && inst->writes_flag()) { > @@ -438,22 +438,19 @@ vec4_visitor::dead_code_eliminate() > node = prev, prev = prev->prev) { > vec4_instruction *scan_inst = (vec4_instruction *)node; > > - if (scan_inst->has_side_effects()) > - continue; > - > if (inst_writes_flag) { > if (scan_inst->dst.is_null() && scan_inst->writes_flag()) { > scan_inst->remove(); > progress = true; > + continue;
You don't want to be remove()ing in the has_side_effects() case, right? (think removing an atomic inc). > - } else if (scan_inst->dst.file != GRF) { > - continue; > } > > - if (inst->dst.reg == scan_inst->dst.reg) { > + if (inst->dst.file == GRF && > + scan_inst->dst.file == GRF && > + inst->dst.reg == scan_inst->dst.reg) { > int new_writemask = scan_inst->dst.writemask & ~dead_channels; > > progress = try_eliminate_instruction(scan_inst, new_writemask) || Doesn't this change prevent the inst_writes_flag == true case from optimizing?
pgpRK8N7CAl5n.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev