On Mon, May 14, 2018 at 1:22 PM, Jason Ekstrand <[email protected]> wrote:
> On Mon, May 14, 2018 at 1:05 PM, Samuel Pitoiset < > [email protected]> 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. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106090 >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106504 >> CC: 18.0 18.1 <[email protected]> >> Signed-off-by: Samuel Pitoiset <[email protected]> >> --- >> src/compiler/spirv/vtn_cfg.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c >> index e7d2f9ea61..28554e8c72 100644 >> --- a/src/compiler/spirv/vtn_cfg.c >> +++ b/src/compiler/spirv/vtn_cfg.c >> @@ -374,6 +374,13 @@ 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); >> >> + /* 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. >> + */ >> + if (new_loop_break == loop_cont) >> + return; >> > > I think this is mostly correct. However, I think what we really want is > to call vtn_get_branch_type() and bail if the returnd branch type is not > vtn_branch_type_none. Possibly with an assert like we have in switch case > handling. > In particular, I think loop continues and none are probably the only valid branch types. It's possible that a switch case fall-through would also be valid but I haven't thought about it long enough. That should be considered. --Jason > > >> + >> block = new_loop_break; >> continue; >> } >> -- >> 2.17.0 >> >> _______________________________________________ >> mesa-dev mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
