[Mesa-dev] [PATCH] gallium: add PIPE_SHADER_CAP_SUBROUTINES

Jose Fonseca jfonseca at vmware.com
Mon Nov 22 01:54:18 PST 2010


I have no objection with the change FWIW.

I agree that given that there is no shared compiler infrastructure for gallium drivers exposing the right capabilities to state tracker is the good compromise.

Concerning replacing TGSI with something else, I've said it before, but it doesn't hurt re-state it: yes, it has become obvious that gallium needs a compiler insfrastructure, and I would have no objection with TGSI being replaced with GLSL IR or LLVM IR, provided that this IR is a superset of TGSI and the infrastructure as portable as gallium. 

That said, we have no immediate plans to carry such work. We'd be happy to provide feedback/assistance to anybody in the community though. LunarG already showed interest in developing LLVM IR, and although challenging the prospects of using LLVM is also enticing. Intel's focus so far has been in the Mesa classic, but if you/others want to look into GLSL2 in gallium it looks a worthy project as well.

I didn't have opportunity to look at GLSL2 to closely, but technically GLSL2 seems a fine piece of software. The only dislike I have with GLSL2 is the LGPL dependency of talloc. IMO baking this dependency into gallium interfaces would  hampers certain business models around gallium -- it becomes very difficult to embed gallium in custom software/hardware proprietary applications while respecting LGPL terms.  So for me to support GLSL2 IR the talloc dependency would have to be severed.

Jose


________________________________________
From: mesa-dev-bounces+jfonseca=vmware.com at lists.freedesktop.org [mesa-dev-bounces+jfonseca=vmware.com at lists.freedesktop.org] On Behalf Of Marek Olšák [maraeo at gmail.com]
Sent: Saturday, November 20, 2010 13:04
To: mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] gallium: add PIPE_SHADER_CAP_SUBROUTINES

Hi,

Sorry that I am asking this but if a patch appears to be ignored on ML, should I push it to master anyway or should I ping you?

The reason I'd like to add this CAP is to leverage what has already been implemented in GLSL2.

Ideally I'd like to use GLSL2 IR in gallium drivers directly without messing with TGSI that has no compiler infrastructure, but I can't. TGSI takes so much away (e.g. the ability to use GLSL2 lowering passes and generate code directly from that IR) and gives nothing in return. GLSL doesn't have to be just a shading language in OpenGL, it can even be a very good, API-agnostic high-level language, something that Gallium lacks. And the GLSL2 compiler is self-contained anyway, so I don't see why Gallium drivers couldn't use it. Just some thoughts FWIW.

Best regards,
Marek

On Sun, Nov 14, 2010 at 3:48 PM, Marek Olšák <maraeo at gmail.com<mailto:maraeo at gmail.com>> wrote:
This fixes piglit/glsl-vs-main-return for the drivers which don't
support RET (i915g, r300g, r600g, svga).

ir_to_mesa does not currently generate subroutines, but it's a matter of time
till it's added. It would then break all the drivers which don't implement
them, so this CAP makes sense.

Signed-off-by: Marek Olšák <maraeo at gmail.com<mailto:maraeo at gmail.com>>
---
 src/gallium/auxiliary/tgsi/tgsi_exec.h |    2 ++
 src/gallium/drivers/i915/i915_screen.c |    2 ++
 src/gallium/drivers/i965/brw_screen.c  |    2 ++
 src/gallium/drivers/nv50/nv50_screen.c |    2 ++
 src/gallium/drivers/nvfx/nvfx_screen.c |    4 ++++
 src/gallium/drivers/r300/r300_screen.c |    4 ++++
 src/gallium/drivers/r600/r600_pipe.c   |    2 ++
 src/gallium/drivers/svga/svga_screen.c |    4 ++++
 src/gallium/include/pipe/p_defines.h   |    1 +
 src/mesa/state_tracker/st_extensions.c |    4 ++--
 10 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.h b/src/gallium/auxiliary/tgsi/tgsi_exec.h
index 7b07778..b5ebbfb 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.h
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.h
@@ -388,6 +388,8 @@ tgsi_exec_get_shader_param(enum pipe_shader_cap param)
   case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
   case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
      return 1;
+   case PIPE_SHADER_CAP_SUBROUTINES:
+      return 1;
   default:
      return 0;
   }
diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c
index 312847f..b9e5fc3 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -183,6 +183,8 @@ i915_get_shader_param(struct pipe_screen *screen, unsigned shader, enum pipe_sha
      case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
      case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
         return 1;
+      case PIPE_SHADER_CAP_SUBROUTINES:
+         return 0;
      default:
         assert(0);
         return 0;
diff --git a/src/gallium/drivers/i965/brw_screen.c b/src/gallium/drivers/i965/brw_screen.c
index 57160eb..29486f5 100644
--- a/src/gallium/drivers/i965/brw_screen.c
+++ b/src/gallium/drivers/i965/brw_screen.c
@@ -240,6 +240,8 @@ brw_get_shader_param(struct pipe_screen *screen, unsigned shader, enum pipe_shad
      case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
      case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
          return 1;
+      case PIPE_SHADER_CAP_SUBROUTINES:
+          return 1;
      default:
         assert(0);
         return 0;
diff --git a/src/gallium/drivers/nv50/nv50_screen.c b/src/gallium/drivers/nv50/nv50_screen.c
index 51eab3a..49522b7 100644
--- a/src/gallium/drivers/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nv50/nv50_screen.c
@@ -176,6 +176,8 @@ nv50_screen_get_shader_param(struct pipe_screen *pscreen, unsigned shader,
       case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
       case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
               return 1;
+       case PIPE_SHADER_CAP_SUBROUTINES:
+               return 0;
       default:
               return 0;
       }
diff --git a/src/gallium/drivers/nvfx/nvfx_screen.c b/src/gallium/drivers/nvfx/nvfx_screen.c
index 8bf0907..33ed654 100644
--- a/src/gallium/drivers/nvfx/nvfx_screen.c
+++ b/src/gallium/drivers/nvfx/nvfx_screen.c
@@ -123,6 +123,8 @@ nvfx_screen_get_shader_param(struct pipe_screen *pscreen, unsigned shader, enum
               case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
               case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
                       return 0;
+               case PIPE_SHADER_CAP_SUBROUTINES:
+                       return screen->use_nv4x ? 1 : 0;
               default:
                       break;
               }
@@ -161,6 +163,8 @@ nvfx_screen_get_shader_param(struct pipe_screen *pscreen, unsigned shader, enum
                       return 0;
               case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
                       return 1;
+               case PIPE_SHADER_CAP_SUBROUTINES:
+                       return 1;
               default:
                       break;
               }
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 37563b5..b4b112e 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -212,6 +212,8 @@ static int r300_get_shader_param(struct pipe_screen *pscreen, unsigned shader, e
        case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
        case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
            return 0;
+        case PIPE_SHADER_CAP_SUBROUTINES:
+            return 0;
        }
        break;
    case PIPE_SHADER_VERTEX:
@@ -245,6 +247,8 @@ static int r300_get_shader_param(struct pipe_screen *pscreen, unsigned shader, e
            return 0;
        case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
            return 1;
+        case PIPE_SHADER_CAP_SUBROUTINES:
+            return 0;
        default:
            break;
        }
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 2a113f0..f46fac0 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -375,6 +375,8 @@ static int r600_get_shader_param(struct pipe_screen* pscreen, unsigned shader, e
       case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
       case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
               return 1;
+       case PIPE_SHADER_CAP_SUBROUTINES:
+               return 0;
       default:
               return 0;
       }
diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
index af99c41..666b498 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -237,6 +237,8 @@ static int svga_get_shader_param(struct pipe_screen *screen, unsigned shader, en
      case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
      case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
         return 0;
+      case PIPE_SHADER_CAP_SUBROUTINES:
+         return 0;
      }
      break;
   case PIPE_SHADER_VERTEX:
@@ -276,6 +278,8 @@ static int svga_get_shader_param(struct pipe_screen *screen, unsigned shader, en
         return 0;
      case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
         return 1;
+      case PIPE_SHADER_CAP_SUBROUTINES:
+         return 0;
      default:
         break;
      }
diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
index 6cca301..dacabed 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -489,6 +489,7 @@ enum pipe_shader_cap
   PIPE_SHADER_CAP_INDIRECT_OUTPUT_ADDR,
   PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR,
   PIPE_SHADER_CAP_INDIRECT_CONST_ADDR,
+   PIPE_SHADER_CAP_SUBROUTINES, /* BGNSUB, ENDSUB, CAL, RET */
 };

 /**
diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
index 1327491..d4f0df8 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -169,9 +169,9 @@ void st_init_limits(struct st_context *st)

      /* TODO: make these more fine-grained if anyone needs it */
      options->EmitNoIfs = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_MAX_CONTROL_FLOW_DEPTH);
-      options->EmitNoFunctions = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_MAX_CONTROL_FLOW_DEPTH);
      options->EmitNoLoops = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_MAX_CONTROL_FLOW_DEPTH);
-      options->EmitNoMainReturn = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_MAX_CONTROL_FLOW_DEPTH);
+      options->EmitNoFunctions = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_SUBROUTINES);
+      options->EmitNoMainReturn = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_SUBROUTINES);

      options->EmitNoCont = !screen->get_shader_param(screen, i, PIPE_SHADER_CAP_TGSI_CONT_SUPPORTED);

--
1.7.0.4




More information about the mesa-dev mailing list