[Mesa-stable] [Mesa-dev] [PATCH v2] spirv: fix visiting inner loops with same break/continue block

[Samuel Pitoiset]

On 05/15/2018 05:56 PM, Jason Ekstrand wrote:
> I don't think this does the right thing in the switch fall-through case 
> but it will at least fail with a reasonable assert instead of just doing 
> the wrong thing.

Yeah, I'm not sure but that could be fixed later if we hit the 
situation. Thanks for reviewing it!

> 
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net 
> <mailto:jason at jlekstrand.net>>
> 
> On Tue, May 15, 2018 at 3:00 AM, Samuel Pitoiset 
> <samuel.pitoiset at gmail.com <mailto:samuel.pitoiset at gmail.com>> wrote:
> 
>     We should stop walking through the CFG when the inner loop's
>     break block ends up as the same block as the outer loop's
>     continue block because we are already going to visit it.
> 
>     This fixes the following assertion which ends up by crashing
>     in RADV or ANV:
> 
>     SPIR-V parsing FAILED:
>     In file ../src/compiler/spirv/vtn_cfg.c:381
>     block->node.link.next == NULL
>     0 bytes into the SPIR-V binary
> 
>     This also fixes a crash with a camera shader from SteamVR.
> 
>     v2: make use of vtn_get_branch_type() and add an assertion
> 
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106090
>     <https://bugs.freedesktop.org/show_bug.cgi?id=106090>
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106504
>     <https://bugs.freedesktop.org/show_bug.cgi?id=106504>
>     CC: 18.0 18.1 <mesa-stable at lists.freedesktop.org
>     <mailto:mesa-stable at lists.freedesktop.org>>
>     Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com
>     <mailto:samuel.pitoiset at gmail.com>>
>     ---
>       src/compiler/spirv/vtn_cfg.c | 13 +++++++++++++
>       1 file changed, 13 insertions(+)
> 
>     diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
>     index e7d2f9ea61..ad4374112e 100644
>     --- a/src/compiler/spirv/vtn_cfg.c
>     +++ b/src/compiler/spirv/vtn_cfg.c
>     @@ -374,6 +374,19 @@ vtn_cfg_walk_blocks(struct vtn_builder *b,
>     struct list_head *cf_list,
>                vtn_cfg_walk_blocks(b, &loop->cont_body, new_loop_cont,
>     NULL, NULL,
>                                    new_loop_break, NULL, block);
> 
>     +         enum vtn_branch_type branch_type =
>     +            vtn_get_branch_type(b, new_loop_break, switch_case,
>     switch_break,
>     +                                loop_break, loop_cont);
>     +
>     +         if (branch_type != vtn_branch_type_none) {
>     +            /* Stop walking through the CFG when this inner loop's
>     break block
>     +             * ends up as the same block as the outer loop's
>     continue block
>     +             * because we are already going to visit it.
>     +             */
>     +            vtn_assert(branch_type == vtn_branch_type_loop_continue);
>     +            return;
>     +         }
>     +
>                block = new_loop_break;
>                continue;
>             }
>     -- 
>     2.17.0
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> 
>