nir/from_ssa: Only re-locate values that are destinations

There is an optimization in the parallel copy algorithm where, after a
copy has been performed, we can treat the destination as the new source
for future copies of the same source.  In particular, consider the
following parallel copy: A -> B, C -> A, A -> C.  In this case, after we
have done the A -> B copy, we can make note that the value in A is now
in B and emit the sequence: A -> B, C -> A, B -> C.  This allows us to
resolve the swap cycle between A anc C without allocating a temporary
register because we know B is also a copy of A.

When one of the registers involved is convergent and the other is
divergent, this optimization is problematic because, while convergent to
divergent copies are fine, we can't re-use the divergent copy in later
copies if any of those copies are to a convergent variable.  We could,
but it would require a read_first_invocation which would get messy.  In
In c7fc44f9eb ("nir/from_ssa: Respect and populate divergence
information"), we attempted to deal with this by limiting the rename
optimization to the case where the divergence matched.

The problem is that we did the re-name part whenever the divergence
matched but only marked it as ready if the thing being copied was a
destination.  (We actually left two instances of loc[a] = b, one which
always happened and one which only happened if we also wanted to flag
the source as being ready to use as a destination.)  While this
technically doesn't cause any problems, it may result in more inter-mov
dependencies which hurts instruction scheduling.  For example, if we had
the parallel copy A -> B, A -> C, A -> D, we now end up emitting the
sequence A -> B, B -> C, C -> D which has many more data hazards between
instructions caused by the constant shuffling.

This commit restores the original logic in which we only perform the
rename optimization if the rename would free up a register we will later
use as a destination.  This isn't entirely optimal as it still doesn't
prove that there is a cycle involved first, but it should lead to a
reduction in unnecessary dependencies.

No shader-db changes on SKL or DG2

Fixes: c7fc44f9eb ("nir/from_ssa: Respect and populate divergence information")
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21315>
This commit is contained in:
Faith Ekstrand
2023-02-13 17:32:35 -06:00
committed by Marge Bot
parent 9673502b3b
commit 5afba073c6

View File

@@ -783,11 +783,6 @@ resolve_parallel_copy(nir_parallel_copy_instr *pcopy,
*/
if (nir_src_is_divergent(values[a]) ==
nir_src_is_divergent(values[b])) {
/* If any other copies want a they can find it at b but only if the
* two have the same divergence.
*/
loc[a] = b;
/* If a needs to be filled... */
if (pred[a] != -1) {
/* If any other copies want a they can find it at b */