From 8665e37960f2ad495500a1cf1d8f007beb22b119 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Fri, 18 Aug 2023 09:34:35 -0700 Subject: [PATCH] intel/fs: Don't try to copy propagate into a source again after progress is made If the linked list structure used depended on the list head to know when to terminate, this would be a pretty serious bug. If try_constant_propage or try_copy_propagate make progress, inst->src[i].nr will change. This results in the foreach_in_list using a different list header on later iterations of the loop. This causes two shaders in shader-db and 9 shaders in fossil-db to change. Looking at the code changes, these are cases where there was a copy of a copy that gets propagated. The part that confuses me is the VGRF numbers involved should **not** hash to the same bucket, so it should be impossible to find the original source from the intermediate VGRF. Reviewed-by: Kenneth Graunke Part-of: --- src/intel/compiler/brw_fs_copy_propagation.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp index 8151b7d..00443e0 100644 --- a/src/intel/compiler/brw_fs_copy_propagation.cpp +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp @@ -1181,10 +1181,13 @@ fs_visitor::opt_copy_propagation_local(void *copy_prop_ctx, bblock_t *block, continue; foreach_in_list(acp_entry, entry, &acp[inst->src[i].nr % ACP_HASH_SIZE]) { - if (try_constant_propagate(inst, entry)) + if (try_constant_propagate(inst, entry)) { progress = true; - else if (try_copy_propagate(inst, i, entry)) + break; + } else if (try_copy_propagate(inst, i, entry)) { progress = true; + break; + } } } -- 2.7.4