diff options
author | Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> | 2020-07-31 10:43:55 -0400 |
---|---|---|
committer | Eric Engestrom <eric@engestrom.ch> | 2020-08-05 22:23:21 +0200 |
commit | 8202415ebc736e739f3bb1d3ec18dde9363ecaff (patch) | |
tree | 3c5a34810a12216c8703721c493757135b550515 /src | |
parent | 3be7fca3ee2662599f7bf3205f88bc3114727a32 (diff) |
pan/mdg: Test for SSA before chasing addresses
It's possible an SSA value depends on a register; in this case, chasing
the source would result in a crash as the chase helper in NIR asserts
is_ssa. Instead we should check a priori that all the argments are in
fact SSA, bailing otherwise.
In the piglit shader exhibiting this bug (by looping over the index),
bailing on the ishl instruction is -necessary-. This is not merely us
being cowardly to avoid seeing through the registers; indeed, if we
wrote away the ishl instruction, the shift itself would have to be
stored in a load/store register (r26/r27) which would preclude reading
it in the loop, creating a register allocation failure later in the
compile. So this is the correct solution due to the restricted
semantics.
Closes #3286
Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Reported-by: Icecream95 <ixn@keemail.me>
Fixes: f5401cb8866 ("pan/midgard: Add address analysis framework")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6144>
(cherry picked from commit b2f475251ed9fa8b37cb98372e64d6166d48a089)
Diffstat (limited to 'src')
-rw-r--r-- | src/panfrost/midgard/midgard_address.c | 27 |
1 files changed, 25 insertions, 2 deletions
diff --git a/src/panfrost/midgard/midgard_address.c b/src/panfrost/midgard/midgard_address.c index 7b8e01811d2..c6ba9007ffd 100644 --- a/src/panfrost/midgard/midgard_address.c +++ b/src/panfrost/midgard/midgard_address.c @@ -45,6 +45,20 @@ struct mir_address { unsigned bias; }; +static bool +mir_args_ssa(nir_ssa_scalar s, unsigned count) +{ + nir_alu_instr *alu = nir_instr_as_alu(s.def->parent_instr); + assert(count <= nir_op_infos[alu->op].num_inputs); + + for (unsigned i = 0; i < count; ++i) { + if (!alu->src[i].src.is_ssa) + return false; + } + + return true; +} + /* Matches a constant in either slot and moves it to the bias */ static void @@ -69,6 +83,9 @@ mir_match_iadd(struct mir_address *address, bool first_free) if (!address->B.def || !nir_ssa_scalar_is_alu(address->B)) return; + if (!mir_args_ssa(address->B, 2)) + return; + nir_op op = nir_ssa_scalar_alu_op(address->B); if (op != nir_op_iadd) return; @@ -96,6 +113,9 @@ mir_match_u2u64(struct mir_address *address) if (!address->B.def || !nir_ssa_scalar_is_alu(address->B)) return; + if (!mir_args_ssa(address->B, 1)) + return; + nir_op op = nir_ssa_scalar_alu_op(address->B); if (op != nir_op_u2u64) return; nir_ssa_scalar arg = nir_ssa_scalar_chase_alu_src(address->B, 0); @@ -112,6 +132,9 @@ mir_match_ishl(struct mir_address *address) if (!address->B.def || !nir_ssa_scalar_is_alu(address->B)) return; + if (!mir_args_ssa(address->B, 2)) + return; + nir_op op = nir_ssa_scalar_alu_op(address->B); if (op != nir_op_ishl) return; nir_ssa_scalar op1 = nir_ssa_scalar_chase_alu_src(address->B, 0); @@ -134,14 +157,14 @@ mir_match_mov(struct mir_address *address) if (address->A.def && nir_ssa_scalar_is_alu(address->A)) { nir_op op = nir_ssa_scalar_alu_op(address->A); - if (op == nir_op_mov) + if (op == nir_op_mov && mir_args_ssa(address->A, 1)) address->A = nir_ssa_scalar_chase_alu_src(address->A, 0); } if (address->B.def && nir_ssa_scalar_is_alu(address->B)) { nir_op op = nir_ssa_scalar_alu_op(address->B); - if (op == nir_op_mov) + if (op == nir_op_mov && mir_args_ssa(address->B, 1)) address->B = nir_ssa_scalar_chase_alu_src(address->B, 0); } } |