[Bug 111075] Processing of SPIR-V shader causes device hang, sometimes leading to system reboot

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=111075

--- Comment #3 from Danylo <danylo.piliaiev at gmail.com> ---
I'm unable to come up with any seemingly good solution at the moment so here
what I have found:

The hang happens due to the usage of uninitialized memory in calculations of
loop terminator.
Bisected commit (8fb8ebfbb05d3323481c8ba6d320b3a3580bad99) just makes other
optimization go wrong.

The usage of uninitialized memory starts with nir_lower_regs_to_ssa_impl which
happens after opt_if_cf_list. During the nir_lower_regs_to_ssa_impl have (pay
attention to r23):

> decl_reg vec1 32 r23
> ...
> /* succs: block_1 */
> loop {
>     block block_1:
>     /* preds: block_0 block_12 */
>     ...
>     /* succs: block_5 */
>     loop {
>         block block_5:
>         /* preds: block_4 block_9 block_11 */
>         vec1 32 ssa_32 = phi block_4: ssa_5, block_9: r23, block_11: ssa_168
>         r23 = phi block_4: ssa_5, block_9: r27, block_11: ssa_170
>         ...
>         /* succs: block_6 block_7 */
>         if r25 {
>             block block_6:
>             /* preds: block_5 */
>             ...
>             break
>             /* succs: block_12 */
>         } else {
>             block block_7:
>             /* preds: block_5 */
>             /* succs: block_8 */
>         }
>         block block_8:
>         /* preds: block_7 */
>         vec1 32 ssa_47 = i2f32 r23
>         /* succs: block_9 block_10 */
>         if r26 {
>             block block_9:
>             /* preds: block_8 */
>             r27 = iadd r23, ssa_5
>             continue
>             /* succs: block_5 */
>         } else {
>             block block_10:
>             /* preds: block_8 */
>             break
>             /* succs: block_12 */
>         }
>         block block_11:
>         /* preds: */
>         /* succs: block_5 */
>     }
>     block block_12:
>     /* preds: block_6 block_10 */
>     ...
>     /* succs: block_1 */
> }

r23 is used in calculations of ssa_32 thus when we are calling
nir_phi_builder_value_get_block_def with block_9 and r23 in rewrite_src we are
trying to find the closest ssa_def by traversing the dominance tree.

ssa_def is found in block_5 - the only place where r23 is assigned, however
it is NEEDS_PHI since we haven't processed the r23 assignment yet. So empty phi
is created, placed in the list and then added to all blocks between block_9 and
block_5 in dominance tree.

Next, the r23 assignment is processed, nir_phi_builder_value_set_block_def is
called with new ssa for this block however the later blocks e.g. block_8 still
has empty phi for r23. So when assignment to ssa_47 is processed it fetches the
empty phi which corresponds to this block for r23, now ssa_47 uses incorrect
source:

> block block_5:
> /* preds: block_4 block_9 block_11 */
> vec1 32 ssa_32 = phi block_4: ssa_5, block_9: ssa_4294967295, block_11: ssa_168
> vec1 32 ssa_223 = phi block_4: ssa_5, block_9: ssa_4294967295, block_11: ssa_170
> /* succs: block_6 block_7 */
> if ssa_225 {
>     block block_6:
>     /* preds: block_5 */
>     break
>     /* succs: block_12 */
> } else {
>     block block_7:
>     /* preds: block_5 */
>     /* succs: block_8 */
> }
> block block_8:
> /* preds: block_7 */
> vec1 32 ssa_47 = i2f32 ssa_4294967295

Later in nir_phi_builder_finish "virtual" phi blocks are materialized not
taking into account the fact that r23 was assigned. So we get:

>block block_0:
>vec1 32 ssa_248 = undefined
>loop {
>	block block_1:
>    vec1 32 ssa_249 = phi block_0: ssa_248, block_12: ssa_223
>
>    loop {
>        block block_5:
>        /* preds: block_4 block_9 block_11 */
>        vec1 32 ssa_247 = phi block_4: ssa_249, block_9: ssa_247, block_11: ssa_246
>        vec1 32 ssa_32 = phi block_4: ssa_5, block_9: ssa_247, block_11: ssa_168
>        vec1 32 ssa_223 = phi block_4: ssa_5, block_9: ssa_255, block_11: ssa_170
>        /* succs: block_6 block_7 */
>        if ssa_225 {
>            block block_6:
>            /* preds: block_5 */
>            break
>            /* succs: block_12 */
>        } else {
>            block block_7:
>            /* preds: block_5 */
>            /* succs: block_8 */
>        }
>        block block_8:
>        /* preds: block_7 */
>        vec1 32 ssa_47 = i2f32 ssa_247

In this last NIR ssa_47 uses the value which is essentially ssa_248 during the
first loop iteration and is undefined.

I thought that the initial NIR arrangement violates some unwritten rule but
commenting out last block in nir_phi_builder_value_get_block_def resulted in a
correct shader and no other code has issues with such NIR.

I'm unsure if it should take into account the situation described above in some
way or if such NIR arrangement shouldn't happen.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-3d-bugs/attachments/20190712/6deb2420/attachment.html>