i965/fs: fix copy propagation of partially invalidated entries

We were not invalidating entries with a src that reads more than one register
when we find writes that overwrite any register read by entry->src after
the first. This leads to incorrect copy propagation because we re-use
entries from the ACP that have been partially invalidated. Same thing for
entries with a dst that writes to more than one register.

v2 (Sam):
- Improve code by defining regions_overlap() and using it instead of a
loop (Curro).

Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This commit is contained in:
Iago Toral Quiroga 2016-03-11 14:35:07 +01:00 committed by Samuel Iglesias Gonsálvez
parent ea1ef49a16
commit cf375a3333

View file

@ -44,6 +44,7 @@ struct acp_entry : public exec_node {
fs_reg dst;
fs_reg src;
uint8_t regs_written;
uint8_t regs_read;
enum opcode opcode;
bool saturate;
};
@ -702,6 +703,14 @@ can_propagate_from(fs_inst *inst)
!inst->is_partial_write());
}
inline bool
regions_overlap(const fs_reg &r, unsigned n, const fs_reg &s, unsigned m)
{
return r.file == s.file && r.nr == s.nr &&
!(r.reg_offset + n < s.reg_offset ||
s.reg_offset + m < r.reg_offset);
}
/* Walks a basic block and does copy propagation on it using the acp
* list.
*/
@ -728,18 +737,26 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block,
/* kill the destination from the ACP */
if (inst->dst.file == VGRF) {
foreach_in_list_safe(acp_entry, entry, &acp[inst->dst.nr % ACP_HASH_SIZE]) {
if (inst->overwrites_reg(entry->dst)) {
entry->remove();
}
}
if (regions_overlap(entry->dst, entry->regs_written,
inst->dst, inst->regs_written)) {
entry->remove();
break;
}
}
/* Oops, we only have the chaining hash based on the destination, not
* the source, so walk across the entire table.
*/
for (int i = 0; i < ACP_HASH_SIZE; i++) {
foreach_in_list_safe(acp_entry, entry, &acp[i]) {
if (inst->overwrites_reg(entry->src))
/* Make sure we kill the entry if this instruction overwrites
* _any_ of the registers that it reads
*/
if (regions_overlap(entry->src, entry->regs_read,
inst->dst, inst->regs_written)) {
entry->remove();
continue;
}
}
}
}
@ -748,10 +765,11 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block,
* operand of another instruction, add it to the ACP.
*/
if (can_propagate_from(inst)) {
acp_entry *entry = ralloc(copy_prop_ctx, acp_entry);
entry->dst = inst->dst;
entry->src = inst->src[0];
acp_entry *entry = ralloc(copy_prop_ctx, acp_entry);
entry->dst = inst->dst;
entry->src = inst->src[0];
entry->regs_written = inst->regs_written;
entry->regs_read = inst->regs_read(0);
entry->opcode = inst->opcode;
entry->saturate = inst->saturate;
acp[entry->dst.nr % ACP_HASH_SIZE].push_tail(entry);
@ -769,6 +787,7 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block,
entry->dst.reg_offset = offset;
entry->src = inst->src[i];
entry->regs_written = regs_written;
entry->regs_read = inst->regs_read(i);
entry->opcode = inst->opcode;
if (!entry->dst.equals(inst->src[i])) {
acp[entry->dst.nr % ACP_HASH_SIZE].push_tail(entry);