[Mesa-dev] [PATCH 02/16] radeonsi: Initial geometry shader support

[Marek Olšák]

On Fri, Jan 24, 2014 at 10:19 PM, Marek Olšák <maraeo at gmail.com> wrote:
> 1) Do we really have to have so much code inside #if...#endif? Can we
> always compile everything and just not set the pipe_context functions
> if LLVM < 3.5 and the same for get_(shader_)param? You can even do it
> without the preprocessor, e.g. if (HAVE_LLVM >= 0x0305) ...
>
> 2) Sooner or later we'll have GL_ARB_shader_storage_buffer_object,
> which adds shader read-write resources. Maybe it would be better to
> put the ring buffers in a new array of descriptors and leave the
> constant buffers with the READ-only usage. The new array of
> descriptors could later be used for OpenGL read-write resources in
> addition to being used for ring buffers.

Actually, my original plan was to put stream-out buffers after OpenGL
shader-storage buffers. We'll probably want to use the same array for
OpenGL shader atomic counters as well.

3) In si_init_gs_rings:
- could you please use readable decimal numbers for specifying the
sizes? Like 1024 * 1024 * ...
- could you please add an assertion to make sure the function is not
called twice to prevent potential memory leaks
- isn't 64 MB too many for a ring buffer?

4) If the GS is disabled, GS_OUT_PRIM_TYPE should be compatible with
the draw_info primitive for transform feedback to work properly. It
was a requirement on R600-Evergreen. Not sure if it applies to SI too.

Please bear with me, I still haven't fully reviewed the whole file
si_shader.c, but it's too late to do it today.

Marek