Bug 84104 - [SNB] Ring hang with geometry shaders
Summary: [SNB] Ring hang with geometry shaders
Status: NEW
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium major
Assignee: Iago Toral
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-19 20:31 UTC by Jason Ekstrand
Modified: 2016-11-03 22:08 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Ekstrand 2014-09-19 20:31:31 UTC
The following two piglit tests cause a ring hang with the new SNB goemetry shader code:

spec/glsl-1.50/execution/geometry/primitive-id-restart GL_TRIANGLE_STRIP_ADJACENCY ffs

spec/glsl-1.50/execution/geometry/primitive-id-restart GL_TRIANGLE_STRIP_ADJACENCY other
Comment 1 Kenneth Graunke 2014-09-21 00:50:44 UTC
Our use of "Dual OWord Block" messages appears to be confusing the simulator; the second offset is not set to a proper value, so it usually ends up being out-of-range and tripping assertions.  We may want to use a different kind of message.

But, that doesn't appear to be the real problem.  Tentatively, it looks like the clipper is hitting an assertion about an unknown primitive topology exiting the GS.

On Gen6, it appears that we have to set the output topology in the GS URB Write message header.  I am not convinced that we're doing that correctly in all cases.  I noticed we zero out the input topology field in g0 right away - which should be fine, because we have the output topology as prog_data->output_topology.  We attempt to set that properly, but I'm not sure if we set it on every vertex - it looks like we may only set it on the first one.  Perhaps someone can sanity check this.

That said, I'm also seeing utterly bizarre values for topology in the simulator dump output - in the "primitive-id-restart GL_TRIANGLE_STRIP_ADJACENCY" test, it sure looks like we're getting 3DPRIM_QUADSTRIP going from the VS->GS, and again going from GS->CL (which is unsupported, and will trigger an assert).

Maybe my analysis is wrong; I can't imagine how that could be.
Comment 2 Iago Toral 2014-09-22 06:06:29 UTC
(In reply to comment #0)
> The following two piglit tests cause a ring hang with the new SNB goemetry
> shader code:
> 
> spec/glsl-1.50/execution/geometry/primitive-id-restart
> GL_TRIANGLE_STRIP_ADJACENCY ffs
> 
> spec/glsl-1.50/execution/geometry/primitive-id-restart
> GL_TRIANGLE_STRIP_ADJACENCY other

I assume this happens without this commit in piglit, right?

commit f43d2550178ddf75aa70983df73adb26c5ef5352
Author: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Date:   Wed Sep 17 10:58:44 2014 +0200

    glsl-1.50-geometry-primitive-id-restart: Add workaround for Intel SandyBrige
    

That commit is a work around for this hang and at least in my case and Samuel's it is sufficient to avoid running into it. Is that your case or are you hitting this even with that commit in piglit?
Comment 3 Iago Toral 2014-09-22 06:13:06 UTC
(In reply to comment #1)
> Our use of "Dual OWord Block" messages appears to be confusing the
> simulator; the second offset is not set to a proper value, so it usually
> ends up being out-of-range and tripping assertions.  We may want to use a
> different kind of message.

Right. For the record, the docs say that the hardware checks for out-of-range writes and does not execute them.

> But, that doesn't appear to be the real problem.  Tentatively, it looks like
> the clipper is hitting an assertion about an unknown primitive topology
> exiting the GS.

mmm... that's interesting.

> On Gen6, it appears that we have to set the output topology in the GS URB
> Write message header.  I am not convinced that we're doing that correctly in
> all cases.  I noticed we zero out the input topology field in g0 right away
> - which should be fine, because we have the output topology as
> prog_data->output_topology.  We attempt to set that properly, but I'm not
> sure if we set it on every vertex - it looks like we may only set it on the
> first one.  Perhaps someone can sanity check this.

Sure, let me have a look and I'll update here.

> That said, I'm also seeing utterly bizarre values for topology in the
> simulator dump output - in the "primitive-id-restart
> GL_TRIANGLE_STRIP_ADJACENCY" test, it sure looks like we're getting
> 3DPRIM_QUADSTRIP going from the VS->GS, and again going from GS->CL (which
> is unsupported, and will trigger an assert).
>
> Maybe my analysis is wrong; I can't imagine how that could be.
Comment 4 Iago Toral 2014-09-22 06:52:40 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Our use of "Dual OWord Block" messages appears to be confusing the
> > simulator; the second offset is not set to a proper value, so it usually
> > ends up being out-of-range and tripping assertions.  We may want to use a
> > different kind of message.
> 
> Right. For the record, the docs say that the hardware checks for
> out-of-range writes and does not execute them.
> 
> > But, that doesn't appear to be the real problem.  Tentatively, it looks like
> > the clipper is hitting an assertion about an unknown primitive topology
> > exiting the GS.
> 
> mmm... that's interesting.
> 
> > On Gen6, it appears that we have to set the output topology in the GS URB
> > Write message header.  I am not convinced that we're doing that correctly in
> > all cases.  I noticed we zero out the input topology field in g0 right away
> > - which should be fine, because we have the output topology as
> > prog_data->output_topology.  We attempt to set that properly, but I'm not
> > sure if we set it on every vertex - it looks like we may only set it on the
> > first one.  Perhaps someone can sanity check this.
> 
> Sure, let me have a look and I'll update here.

Fore reference, the geometry shader used by this test is this:

#version 150
layout(triangles_adjacency) in;
layout(points, max_vertices = 1) out;
out int primitive_id;
void main()
{
  primitive_id = gl_PrimitiveIDIn;
  EmitVertex();
}


Since this particular test has an output type of points, we just set _3DPRIM_POINTLIST directly on every vertex we see. We always set the output topology type on every vertex anyway, whether the output is points or not:

gen6_gs_visitor::visit(ir_emit_vertex *)
{
(...)
   if (c->gp->program.OutputType == GL_POINTS) {
      /* If we are outputting points, then every vertex has PrimStart and
       * PrimEnd set.
       */
      emit(MOV(dst, (_3DPRIM_POINTLIST << URB_WRITE_PRIM_TYPE_SHIFT) |
               URB_WRITE_PRIM_START | URB_WRITE_PRIM_END));
      emit(ADD(dst_reg(this->prim_count), this->prim_count, 1u));
   } else {
      /* Otherwise, we can only set the PrimStart flag, which we have stored
       * in the first_vertex register. We will have to wait until we execute
       * EndPrimitive() or we end the thread to set the PrimEnd flag on a
       * vertex.
       */
      emit(OR(dst, this->first_vertex,
              (c->prog_data.output_topology << URB_WRITE_PRIM_TYPE_SHIFT)));
      emit(MOV(dst_reg(this->first_vertex), 0u));
   }
(...)
}

The comment in the else branch refers to another flag that we only need to set
for the last vertex in a primitive, but as you can see, the output topology
type is always set.

> > That said, I'm also seeing utterly bizarre values for topology in the
> > simulator dump output - in the "primitive-id-restart
> > GL_TRIANGLE_STRIP_ADJACENCY" test, it sure looks like we're getting
> > 3DPRIM_QUADSTRIP going from the VS->GS, and again going from GS->CL (which
> > is unsupported, and will trigger an assert).
> >
> > Maybe my analysis is wrong; I can't imagine how that could be.

In this particular example with points output, 3DPRIM_QUADSTRIP In the GS->CL stage can't happen, since for an output type of points we just write _3DPRIM_POINTLIST as I explain above. That said, it is interesting that you also see 3DPRIM_QUADSTRIP going from the VS to the GS. I'll debug the output of the VS and update here if I see something interesting.
Comment 5 Iago Toral 2014-09-22 08:58:32 UTC
For reference, this is all that I could find above this hang so far:
http://lists.freedesktop.org/archives/mesa-dev/2014-August/065726.html

In summary:
  - glDrawElements is used
  - There are repeating indices (at least 8 consecutive repeating indices, or 8 repeating vertices in any subset of 9 consecutive indices)
  - primitive type is GL_TRIANGLE_STRIP_ADJACENCY
  - The hang persists even in the case that the GS is empty and we clip all the vertices (so it really looks like this is unrelated to the code we generate)

I can reproduce the problem with a standalone program that can switch between glDrawArrays and glDrawElements (for glDrawArrays I just pass the same vertex data for all vertices). Both executions generate the exact same code for the VS and GS, but only the version with glDrawElements version hangs...

So based on this, it would really look like the problem is not in the geometry shader implementation, but something else.

Kenneth, here are two AUB traces I obtained using this program. One for glDrawElements and one for  glDrawArrays. Could you run both through the simulator and see if you spot any differences? Maybe that can give us a clue....

http://people.igalia.com/itoral/intel.hangs_with_elements.aub
http://people.igalia.com/itoral/intel.works_with_arrays.aub
Comment 6 Iago Toral 2014-09-22 13:42:03 UTC
I think the case that this still hangs even in the presence of an empty GS is remarkable. So If I use this GS:

#version 150
layout(triangles_adjacency) in;
layout(points, max_vertices=1) out;
void main()
{
}

the GS stage will not emit any vertices. I have even modified the implementation to eliminate the loop that handles the URB writes so that the native code we generate is trivial (the loop would not be executed anyway, but thisway  we see that the resulting program is trivial in this case), and the hang persists. With this shader, and removing the emission loop, the code generated in with INTEL_DEBUG=gs is reduced to this and still hangs:

   START B0
   clear r0.2
mov(1)   g0.2<1>UD    0x00000000UD    { align1 WE_all compacted };
   gen6 prolog
mov(8)   m1<1>UD      g0<4,4,1>UD     { align16 WE_all 1Q compacted };
   gen6 thread end: EOT
send(8)  null         m1<4,4,1>F
         urb 0 urb_write interleave complete mlen 1 rlen 0 { align16 1Q EOT };
   END B0

I think this discards that the problem is related to the output primitive type we emit in the GS stage and generally, any aspect of the code that we generate for the GS stage.

Kenneth, I recorded an AUB trace for this case too, since in this case we are not generating any DUAL OWORD BLOCK messages or even generating any output in the GS at all, I wonder if the emulator still complains about something... if it does it might be more useful:

http://people.igalia.com/itoral/intel.hangs_empty_gs.aub


Of course, the above setup works fine if any of the conditions that lead to the hang are removed, that is:

- if we use glDrawArrays instead of glDrawElements or
- if we render a primitive other than GL_TRIANGLE_STRIP_ADJACENCY or
- if the list of indices we use with glDrawElements does not have 8 consecutive repeating vertices or at least 8 repeated vertices in any subset of 9 consecutive indices.
Comment 7 Jason Ekstrand 2014-09-22 19:21:20 UTC
(In reply to comment #2)
> (In reply to comment #0)
> > The following two piglit tests cause a ring hang with the new SNB goemetry
> > shader code:
> > 
> > spec/glsl-1.50/execution/geometry/primitive-id-restart
> > GL_TRIANGLE_STRIP_ADJACENCY ffs
> > 
> > spec/glsl-1.50/execution/geometry/primitive-id-restart
> > GL_TRIANGLE_STRIP_ADJACENCY other
> 
> I assume this happens without this commit in piglit, right?
> 
> commit f43d2550178ddf75aa70983df73adb26c5ef5352
> Author: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
> Date:   Wed Sep 17 10:58:44 2014 +0200
> 
>     glsl-1.50-geometry-primitive-id-restart: Add workaround for Intel
> SandyBrige
>     
> 
> That commit is a work around for this hang and at least in my case and
> Samuel's it is sufficient to avoid running into it. Is that your case or are
> you hitting this even with that commit in piglit?

Yes, it was without that commit.
Comment 8 Iago Toral 2014-09-24 06:34:36 UTC
One last piece of info: setting GEN6_GS_DISCARD_ADJACENCY in the _3DSTATE_GS command does not remove the hang either. I guess this means that the hang occurs before the GS payload is delivered and definitely before any GS program can be executed. I guess this makes sense, since otherwise it would be difficult to explain why this hangs only with glDrawElements.

Based on all the analysis so far, I suspect that the hang is happening at the VS stage when accessing vertex data for GL_TRIANGLE_STRIP_ADJACENCY primitives when repeating indices are provided via glDrawElements.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.