[Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

Pierre Moreau pierre.morrow at free.fr
Wed Feb 24 21:56:41 UTC 2016


On 08:44 PM - Feb 24 2016, Samuel Pitoiset wrote:
> 
> 
> On 02/24/2016 08:30 PM, Pierre Moreau wrote:
> >Hi Samuel,
> >
> >On 06:44 PM - Feb 24 2016, Samuel Pitoiset wrote:
> >>Reduce the amount of duplicated code by re-using
> >>nvc0_program_validate(). While we are at it, change the prototype
> >>to return void and remove nvc0_compute.h which is now useless.
> >
> >Why don't you want to know whether the validation worked or not? If the
> >validation failed, the program has a bug and Nouveau shouldn't try to run it,
> >so allocating buffers that will be unused seems wasteful. Am I missing
> >something here?
> 
> Because it's useless? If the program can't be correctly validated it's going
> to break the universe. If you look at nvc0_gmtyprog_validate() for example
> you will see that I just follow the same design.

I should have checked the other `*_prog_validate()` functions indeed. And I had
forgotten that when those `*_prog_validate()` functions are called, the shaders
have already been transformed from GLSL to TGSI, so user errors in the input
shaders should have been catched already and only translation errors could
occur at this point.

Pierre


Acked-by: Pierre Moreau <pierre.morrow at free.fr>


> >
> >Pierre
> >
> >>
> >>Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> >>---
> >>  src/gallium/drivers/nouveau/Makefile.sources       |  1 -
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c    | 34 ++--------------------
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h    |  9 ------
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_context.h    |  1 +
> >>  .../drivers/nouveau/nvc0/nvc0_shader_state.c       | 15 ++++++++++
> >>  src/gallium/drivers/nouveau/nvc0/nve4_compute.c    |  4 +--
> >>  6 files changed, 20 insertions(+), 44 deletions(-)
> >>  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>
> >>diff --git a/src/gallium/drivers/nouveau/Makefile.sources b/src/gallium/drivers/nouveau/Makefile.sources
> >>index 43ffce6..65f08c7 100644
> >>--- a/src/gallium/drivers/nouveau/Makefile.sources
> >>+++ b/src/gallium/drivers/nouveau/Makefile.sources
> >>@@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
> >>  	nvc0/gm107_texture.xml.h \
> >>  	nvc0/nvc0_3d.xml.h \
> >>  	nvc0/nvc0_compute.c \
> >>-	nvc0/nvc0_compute.h \
> >>  	nvc0/nvc0_compute.xml.h \
> >>  	nvc0/nvc0_context.c \
> >>  	nvc0/nvc0_context.h \
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>index a664aaf..060f59d 100644
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>@@ -23,7 +23,8 @@
> >>   */
> >>
> >>  #include "nvc0/nvc0_context.h"
> >>-#include "nvc0/nvc0_compute.h"
> >>+
> >>+#include "nvc0/nvc0_compute.xml.h"
> >>
> >>  int
> >>  nvc0_screen_compute_setup(struct nvc0_screen *screen,
> >>@@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
> >>     return 0;
> >>  }
> >>
> >>-bool
> >>-nvc0_compute_validate_program(struct nvc0_context *nvc0)
> >>-{
> >>-   struct nvc0_program *prog = nvc0->compprog;
> >>-
> >>-   if (prog->mem)
> >>-      return true;
> >>-
> >>-   if (!prog->translated) {
> >>-      prog->translated = nvc0_program_translate(
> >>-         prog, nvc0->screen->base.device->chipset, &nvc0->base.debug);
> >>-      if (!prog->translated)
> >>-         return false;
> >>-   }
> >>-   if (unlikely(!prog->code_size))
> >>-      return false;
> >>-
> >>-   if (likely(prog->code_size)) {
> >>-      if (nvc0_program_upload_code(nvc0, prog)) {
> >>-         struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> >>-         BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
> >>-         PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
> >>-         return true;
> >>-      }
> >>-   }
> >>-   return false;
> >>-}
> >>-
> >>  static void
> >>  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
> >>  {
> >>@@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
> >>  static bool
> >>  nvc0_compute_state_validate(struct nvc0_context *nvc0)
> >>  {
> >>-   if (!nvc0_compute_validate_program(nvc0))
> >>-      return false;
> >>+   nvc0_compprog_validate(nvc0);
> >>     if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
> >>        nvc0_compute_validate_constbufs(nvc0);
> >>     if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>deleted file mode 100644
> >>index a23f7f3..0000000
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>+++ /dev/null
> >>@@ -1,9 +0,0 @@
> >>-#ifndef NVC0_COMPUTE_H
> >>-#define NVC0_COMPUTE_H
> >>-
> >>-#include "nvc0/nvc0_compute.xml.h"
> >>-
> >>-bool
> >>-nvc0_compute_validate_program(struct nvc0_context *nvc0);
> >>-
> >>-#endif /* NVC0_COMPUTE_H */
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> >>index 7aa4b62..0f1ebb0 100644
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> >>+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> >>@@ -254,6 +254,7 @@ void nvc0_tctlprog_validate(struct nvc0_context *);
> >>  void nvc0_tevlprog_validate(struct nvc0_context *);
> >>  void nvc0_gmtyprog_validate(struct nvc0_context *);
> >>  void nvc0_fragprog_validate(struct nvc0_context *);
> >>+void nvc0_compprog_validate(struct nvc0_context *);
> >>
> >>  void nvc0_tfb_validate(struct nvc0_context *);
> >>
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> >>index 2f46c43..6b02ed5 100644
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> >>+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> >>@@ -28,6 +28,8 @@
> >>  #include "nvc0/nvc0_context.h"
> >>  #include "nvc0/nvc0_query_hw.h"
> >>
> >>+#include "nvc0/nvc0_compute.xml.h"
> >>+
> >>  static inline void
> >>  nvc0_program_update_context_state(struct nvc0_context *nvc0,
> >>                                    struct nvc0_program *prog, int stage)
> >>@@ -257,6 +259,19 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
> >>  }
> >>
> >>  void
> >>+nvc0_compprog_validate(struct nvc0_context *nvc0)
> >>+{
> >>+   struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> >>+   struct nvc0_program *cp = nvc0->compprog;
> >>+
> >>+   if (cp && !nvc0_program_validate(nvc0, cp))
> >>+      return;
> >>+
> >>+   BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
> >>+   PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
> >>+}
> >>+
> >>+void
> >>  nvc0_tfb_validate(struct nvc0_context *nvc0)
> >>  {
> >>     struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> >>index 5c73740..4a4e836 100644
> >>--- a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> >>+++ b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> >>@@ -23,7 +23,6 @@
> >>   */
> >>
> >>  #include "nvc0/nvc0_context.h"
> >>-#include "nvc0/nvc0_compute.h"
> >>  #include "nvc0/nve4_compute.h"
> >>
> >>  #include "codegen/nv50_ir_driver.h"
> >>@@ -306,8 +305,7 @@ nve4_compute_set_tex_handles(struct nvc0_context *nvc0)
> >>  static bool
> >>  nve4_compute_state_validate(struct nvc0_context *nvc0)
> >>  {
> >>-   if (!nvc0_compute_validate_program(nvc0))
> >>-      return false;
> >>+   nvc0_compprog_validate(nvc0);
> >>     if (nvc0->dirty_cp & NVC0_NEW_CP_TEXTURES)
> >>        nve4_compute_validate_textures(nvc0);
> >>     if (nvc0->dirty_cp & NVC0_NEW_CP_SAMPLERS)
> >>--
> >>2.6.4
> >>
> >>_______________________________________________
> >>mesa-dev mailing list
> >>mesa-dev at lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160224/31d280f6/attachment.sig>


More information about the mesa-dev mailing list