[Mesa-dev] [Mesa-stable] [PATCH] i965: Fix JIP to skip over sibling do...while loops.

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> We've apparently always been botching JIP for sequences such as:
>
> do
>     cmp.f0.0 ...
>     (+f0.0) break
>     ...
>     do
>         ...
>     while
>     ...
> while
>
> Because the "do" instruction doesn't actually exist, the inner "while"
> is at the same depth as the "break".  brw_find_next_block_end() thus
> mistook the inner "while" as the end of the loop containing the "break",
> and set the "break" to point to the wrong place.
>
> Only "while" instructions that jump before our instruction are relevant.
> We need to ignore the rest, as they're sibling control flow nodes (or
> children, but this was already handled by the depth == 0 check).
>
> See also commit 1ac1581f3889d5f7e6e231c05651f44fbd80f0b6.
>
> This prevents channel masks from being screwed up, and fixes GPU
> hangs(*) in dEQP-GLES31.functional.shaders.multisample_interpolation.
> interpolate_at_sample.centroid_qualified.multisample_texture_16.
>
> The test ended up executing code with no channels enabled, and that
> code contained FIND_LIVE_CHANNEL, which returned 8 (out of range for
> a SIMD8 program), which then was used in indirect GRF addressing,
> which randomly got a boolean value (0xFFFFFFFF), interpreted it as
> a sample ID, OR'd it into an indirect send message descriptor,
> which corrupted the message length, sending a pixel interpolator
> message with mlen 15, which is illegal.  Whew :)
>
> (*) Technically, the test doesn't GPU hang currently, but only
>     because another bug prevents it from issuing pixel interpolator
>     messages entirely...with that fixed, it hangs.
>
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_eu_emit.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> I really did not expect the 16x MSAA interpolateAt dEQP failures to
> end up being "no pixel interpolator messages at all" combined with
> "nested loops have been broken since 2010"... :)
>
Hah! That's a good one -- And I was wondering some weeks ago how on
earth Gen4-5 users could survive with a hard limit of 7 nested loops,
turns out that even two loop nesting levels have been universally broken
without anyone noticing for years.

> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index cc2876b..d31943d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2689,8 +2689,13 @@ brw_find_next_block_end(struct brw_codegen *p, int start_offset)
>              return offset;
>           depth--;
>           break;
> -      case BRW_OPCODE_ELSE:
>        case BRW_OPCODE_WHILE:
> +         /* If the while doesn't jump before our instruction, it's the end
> +          * of a sibling do...while loop.  Ignore it.
> +          */
> +         if (!while_jumps_before_offset(devinfo, insn, offset, start_offset))
> +            continue;

Did you forget to git-add the definition of this?

> +      case BRW_OPCODE_ELSE:
>        case BRW_OPCODE_HALT:
>           if (depth == 0)
>              return offset;
> -- 
> 2.8.2
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160515/c26237ea/attachment.sig>