nir: only override previous alu during loop analysis if supported

Submitted by Timothy Arceri on March 19, 2019, 1:09 a.m.

Details

Message ID 20190319010936.6427-1-tarceri@itsqueeze.com
State Accepted
Commit 427a6fee439b2df96edc813c56572169385772a6
Headers show
Series "nir: only override previous alu during loop analysis if supported" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Timothy Arceri March 19, 2019, 1:09 a.m.
Users of this function expect alu to be a supported comparision
if the induction variable is not NULL. Since we attempt to
override the return values if the first limit is not a const, we
must make sure we are dealing with a valid comparision before
overriding the alu instruction.

Fixes an unreachable in inverse_comparison() with the game
Assasins Creed Odyssey.

Fixes: 3235a942c16b ("nir: find induction/limit vars in iand instructions")
---
 src/compiler/nir/nir_loop_analyze.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c
index c8304611b28..cb71a55f2f1 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -847,9 +847,11 @@  try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
        !is_var_constant(*limit)) {
       src = iand->src[1].src.ssa;
       if (src->parent_instr->type == nir_instr_type_alu) {
-         *alu = nir_instr_as_alu(src->parent_instr);
-         if (is_supported_terminator_condition(*alu))
+         nir_alu_instr *tmp_alu = nir_instr_as_alu(src->parent_instr);
+         if (is_supported_terminator_condition(tmp_alu)) {
+            *alu = tmp_alu;
             *limit_rhs = get_induction_and_limit_vars(*alu, ind, limit, state);
+         }
       }
    }
 }

Comments

Piglit test to trigger the crash:

https://patchwork.freedesktop.org/patch/292855/

On 19/3/19 12:09 pm, Timothy Arceri wrote:
> Users of this function expect alu to be a supported comparision
> if the induction variable is not NULL. Since we attempt to
> override the return values if the first limit is not a const, we
> must make sure we are dealing with a valid comparision before
> overriding the alu instruction.
> 
> Fixes an unreachable in inverse_comparison() with the game
> Assasins Creed Odyssey.
> 
> Fixes: 3235a942c16b ("nir: find induction/limit vars in iand instructions")
> ---
>   src/compiler/nir/nir_loop_analyze.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c
> index c8304611b28..cb71a55f2f1 100644
> --- a/src/compiler/nir/nir_loop_analyze.c
> +++ b/src/compiler/nir/nir_loop_analyze.c
> @@ -847,9 +847,11 @@ try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
>          !is_var_constant(*limit)) {
>         src = iand->src[1].src.ssa;
>         if (src->parent_instr->type == nir_instr_type_alu) {
> -         *alu = nir_instr_as_alu(src->parent_instr);
> -         if (is_supported_terminator_condition(*alu))
> +         nir_alu_instr *tmp_alu = nir_instr_as_alu(src->parent_instr);
> +         if (is_supported_terminator_condition(tmp_alu)) {
> +            *alu = tmp_alu;
>               *limit_rhs = get_induction_and_limit_vars(*alu, ind, limit, state);
> +         }
>         }
>      }
>   }
>