On 06/30/2015 06:58 PM, Jason Ekstrand wrote: > On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <el...@igalia.com> wrote: >> This patch makes public the is_scalar_shader_stage() method in brw_shader, >> and >> renames it to brw_compiler_is_scalar_shader_stage(). The plan is to later >> reuse it >> in brw_nir, to enable/disable optimization passes depending on the type >> of shader stage. > > I'm not so sure that this is a good plan. It assumes that whether we > are doing a scalar or vec4 compile is based entirely on the shader > stage and some static information (possibly based on environment > variables). Ken and I were talking around the office and we may want > to use both SIMD4x2 and SIMD8 mode for geometry shaders depending on > the number of inputs, etc. This won't work in the given paradigm. >
If I understand correctly, what you propose is having a function to dynamically choose the type of shader (scalar vs. vec4) when compiling the shader, using not only gen and stage, but also actual application data. I think this is a good idea and will allow experimenting with different combinations of shaders with real input data. However, I wonder if we this should be added later after more elaborated thoughts on what exactly do we need and where to plug it. I have been experimenting with a function following the use case you mentioned, choosing shader backend based on inputs to a GS. But honestly it feels like a blind guess from my side to what actually you and Ken have in mind. Current patch basically reuses the function we already have to select the shader, so what I propose is to move forward with it for the moment (adding the missing MESA_SHADER_COMPUTE) and discuss how to extend it to factor in dynamic data; or perhaps you can explain us your proposal with a bit more detail. WDYT? >> The new method accepts a brw_compiler instead of a brw_context. This is done >> for consistency, since the actual info we need (scalar_vs) is in >> brw_compiler, >> and fetching in through brw_content->intelScreen->compiler seems like too >> much indirection. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580 >> --- >> src/mesa/drivers/dri/i965/brw_shader.cpp | 22 ++++++---------------- >> src/mesa/drivers/dri/i965/brw_shader.h | 13 +++++++++++++ >> 2 files changed, 19 insertions(+), 16 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp >> b/src/mesa/drivers/dri/i965/brw_shader.cpp >> index 0b53647..3b99046 100644 >> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp >> @@ -182,19 +182,6 @@ brw_shader_precompile(struct gl_context *ctx, >> return true; >> } >> >> -static inline bool >> -is_scalar_shader_stage(struct brw_context *brw, int stage) >> -{ >> - switch (stage) { >> - case MESA_SHADER_FRAGMENT: >> - return true; >> - case MESA_SHADER_VERTEX: >> - return brw->intelScreen->compiler->scalar_vs; >> - default: >> - return false; >> - } >> -} >> - >> static void >> brw_lower_packing_builtins(struct brw_context *brw, >> gl_shader_stage shader_type, >> @@ -205,7 +192,8 @@ brw_lower_packing_builtins(struct brw_context *brw, >> | LOWER_PACK_UNORM_2x16 >> | LOWER_UNPACK_UNORM_2x16; >> >> - if (is_scalar_shader_stage(brw, shader_type)) { >> + if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler, >> + shader_type)) { >> ops |= LOWER_UNPACK_UNORM_4x8 >> | LOWER_UNPACK_SNORM_4x8 >> | LOWER_PACK_UNORM_4x8 >> @@ -218,7 +206,8 @@ brw_lower_packing_builtins(struct brw_context *brw, >> * lowering is needed. For SOA code, the Half2x16 ops must be >> * scalarized. >> */ >> - if (is_scalar_shader_stage(brw, shader_type)) { >> + if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler, >> + shader_type)) { >> ops |= LOWER_PACK_HALF_2x16_TO_SPLIT >> | LOWER_UNPACK_HALF_2x16_TO_SPLIT; >> } >> @@ -294,7 +283,8 @@ process_glsl_ir(struct brw_context *brw, >> do { >> progress = false; >> >> - if (is_scalar_shader_stage(brw, shader->Stage)) { >> + if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler, >> + shader->Stage)) { >> brw_do_channel_expressions(shader->ir); >> brw_do_vector_splitting(shader->ir); >> } >> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h >> b/src/mesa/drivers/dri/i965/brw_shader.h >> index b2c1a0b..cef2226 100644 >> --- a/src/mesa/drivers/dri/i965/brw_shader.h >> +++ b/src/mesa/drivers/dri/i965/brw_shader.h >> @@ -302,6 +302,19 @@ bool brw_cs_precompile(struct gl_context *ctx, >> struct gl_shader_program *shader_prog, >> struct gl_program *prog); >> >> +static inline bool >> +brw_compiler_is_scalar_shader_stage(struct brw_compiler *compiler, int >> stage) >> +{ >> + switch (stage) { >> + case MESA_SHADER_FRAGMENT: > > You need MESA_SHADER_COMPUTE here too. > >> + return true; >> + case MESA_SHADER_VERTEX: >> + return compiler->scalar_vs; >> + default: >> + return false; >> + } >> +} >> + >> #ifdef __cplusplus >> } >> #endif >> -- >> 2.1.4 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
