<div dir="ltr">On 12 August 2013 13:11, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">fs_copy_prop_dataflow::setup_kills() walks through each basic block's instructions, looking for instructions which overwrite registers used in ACP entries. This would be fine, except that it didn't exclude the instructions which generated the ACP entries in the first place. This meant that every copy was killed by its own generating instruction, making the dataflow analysis useless. </blockquote><div> </div><div>I don't think this is actually a problem, because the bd[b].kills bits are only used to prevent liveness information from being propagated from bd[b].livein to bd[b].liveout, and the fs_copy_prop_dataflow constructor initializes bd[b].liveout with the set of ACP's that are generated from block b. So the only situation in which your patch would allow liveness information to be propagated from bd[b].livein to bd[b].liveout is for bits in bd[b].liveout that are already set. </div><div>Or am I misunderstanding things? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> To fix this, this patch records the generating instruction in the ACP entry. It then skips the kill checks when processing that instruction. Using a void pointer ensures it will only be used for comparisons, and not dereferenced. Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> --- src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp index d8d1546..379605c 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp @@ -42,6 +42,9 @@ namespace { /* avoid conflict with opt_copy_propagation_elements */ struct acp_entry : public exec_node { fs_reg dst; fs_reg src; + + /** MOV instruction which generated this copy/ACP entry. */ + void *inst; }; struct block_data { @@ -146,8 +149,9 @@ fs_copy_prop_dataflow::setup_kills() continue; for (int i = 0; i < num_acp; i++) { - if (inst->overwrites_reg(acp[i]->dst) || - inst->overwrites_reg(acp[i]->src)) { + if (inst != acp[i]->inst && + (inst->overwrites_reg(acp[i]->dst) || + inst->overwrites_reg(acp[i]->src))) { BITSET_SET(bd[b].kill, i); } } @@ -418,6 +422,7 @@ fs_visitor::opt_copy_propagate_local(void *mem_ctx, bblock_t *block, acp_entry *entry = ralloc(mem_ctx, acp_entry); entry->dst = inst->dst; entry->src = inst->src[0]; + entry->inst = inst; acp[entry->dst.reg % ACP_HASH_SIZE].push_tail(entry); } } -- 1.8.3.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>