[Mesa-dev] [PATCH 6/7] i965: ensure execution of fragment shader when fragment shader has atomic buffer access

[Rogovin, Kevin]

One more comment, that I neglected to add: there are other checks for 
_CurrentFragmentProgram to be non-NULL, indeed function 
brw_upload_wm_abo_surface() [file brw_wm_surface_state.c], also 
has a check for it being non-NULL. That function is the emit for
the atom brw_wm_abo_surfaces which is present in both gen7_atoms
and gen8_atoms.

I would argue that _CurrentFragmentProgram can be NULL, given
that other places check for it and that without the check piglit gets
about 30 more crashes.

Sorry for not posting this in the first reply.

 -Kevin

-----Original Message-----
From: Rogovin, Kevin 
Sent: Friday, April 24, 2015 11:22 PM
To: 'Kenneth Graunke'; mesa-dev at lists.freedesktop.org
Cc: mesa-dev at freedesktop.org; currojerez at riseup.net
Subject: RE: [Mesa-dev] [PATCH 6/7] i965: ensure execution of fragment shader when fragment shader has atomic buffer access



> Checking brw->ctx.Shader._CurrentFragmentProgram != NULL is unnecessary.
> There is always a valid pixel shader.  (If the application is using fixed-function, we supply a fragment shader for them.)  Please drop that check.

Without this check(in the Gen7 function/code), about 30 crashes are induced on piglit tests for Gen7; the tests are all using GL fixed function pipeline. I have not run piglit without this check on Gen8 though.

> I thought that UAVs were essentially for Images...I'm not clear why this is needed.  Perhaps Curro can confirm one way or another.

The essential reason is to guarantee that the pixel shader gets invoked by Gen even when all render target surfaces are NULL surfaces. There are other flags one can use, but the UAV seems (to me) the most natural.
 
-Kevin