fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-01-10 12:30:11 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

panfrost: Rework midgard_pair_load_store() to kill the nested foreach loop

Browse source 
mir_foreach_instr_in_block_safe() is based on list_for_each_entry_safe()
which is designed to protect against removal of the current entry, but
removing the entry placed just after the current one will lead to a
use-after-free situation.

Luckily, the midgard_pair_load_store() logic guarantees that the
instruction being removed (if any) is never placed just after ins which
in turn guarantees that the hidden __next variable always points to a
valid object.
Took me a bit of time to realize that this code was safe, so I'm
suggesting to get rid of the inner mir_foreach_instr_in_block_from()
loop and rework the code so that the removed instruction is always the
current one (which is what the list_for_each_entry_safe() API was
initially designed for).

While at it, we also get rid of the unecessary insert(ins)/remove(ins)
dance by simply moving the instruction around.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
This commit is contained in:
Boris Brezillon 2019-08-27 12:36:43 +02:00
parent 0e513ccca4
commit c9bebae287
1 changed files with 32 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

69
src/panfrost/midgard/midgard_schedule.c
View file

@ -657,46 +657,41 @@ schedule_block(compiler_context *ctx, midgard_block *block)
static void
midgard_pair_load_store(compiler_context *ctx, midgard_block *block)
{
midgard_instruction *prev_ldst = NULL;
int search_distance;
mir_foreach_instr_in_block_safe(block, ins) {
if (ins->type != TAG_LOAD_STORE_4) continue;
if (ins->type != TAG_LOAD_STORE_4 && !prev_ldst) continue;
/* We've found a load/store op. Check if next is also load/store. */
midgard_instruction *next_op = mir_next_op(ins);
if (&next_op->link != &block->instructions) {
if (next_op->type == TAG_LOAD_STORE_4) {
/* If so, we're done since we're a pair */
ins = mir_next_op(ins);
continue;
}
/* Maximum search distance to pair, to avoid register pressure disasters */
int search_distance = 8;
/* Otherwise, we have an orphaned load/store -- search for another load */
mir_foreach_instr_in_block_from(block, c, mir_next_op(ins)) {
/* Terminate search if necessary */
if (!(search_distance--)) break;
if (c->type != TAG_LOAD_STORE_4) continue;
/* We can only reorder if there are no sources */
bool deps = false;
for (unsigned s = 0; s < ARRAY_SIZE(ins->src); ++s)
deps |= (c->src[s] != ~0);
if (deps)
continue;
/* We found one! Move it up to pair and remove it from the old location */
mir_insert_instruction_before(ctx, ins, *c);
mir_remove_instruction(c);
break;
}
/* We've found a load/store op. Start searching for another one.
* Maximum search distance to pair, to avoid register pressure disasters
*/
if (!prev_ldst) {
search_distance = 8;
prev_ldst = ins;
continue;
}
/* Already paired. */
if (mir_prev_op(ins) == prev_ldst) {
prev_ldst = NULL;
continue;
}
/* We can only reorder if there are no sources */
bool deps = false;
for (unsigned s = 0; s < ARRAY_SIZE(ins->src); ++s)
deps |= (ins->src[s] != ~0);
/* We found one! Move it up to pair */
if (!deps) {
list_del(&ins->link);
list_add(&ins->link, &prev_ldst->link);
continue;
}
if (!(search_distance--))
prev_ldst = NULL;
}
}