Re: [Mesa-dev] [PATCH] glsl: fix and clean up NV_compute_shader_derivatives support

Timothy Arceri Wed, 01 May 2019 20:23:15 -0700

I feel like I've asked this before but can you please try to add commitproper commit messages. Not adding a proper commit message might saveyou a minute or two but it cost much more than a minute or two for thosetrying to review a patch or that have bisect to the commit, used gitblame, git log etc to find out why a change was made.

I started reviewing the patch when you first sent it but it wasn'timmediately obvious what the fix was vs tidy ups. I didn't want to golooking over specs either, all the relevant information should have beenhere in the commit message.


Thanks,
Tim


On 2/5/19 11:17 am, Marek Olšák wrote:

Ping

On Wed, Apr 24, 2019 at 1:30 PM Marek Olšák <mar...@gmail.com<mailto:mar...@gmail.com>> wrote:


    From: Marek Olšák <marek.ol...@amd.com <mailto:marek.ol...@amd.com>>

    ---
      src/compiler/glsl/builtin_functions.cpp | 78 ++++++++-----------------
      1 file changed, 24 insertions(+), 54 deletions(-)

    diff --git a/src/compiler/glsl/builtin_functions.cpp
    b/src/compiler/glsl/builtin_functions.cpp
    index c8d9e1c9af3..b1ffafa1acf 100644
    --- a/src/compiler/glsl/builtin_functions.cpp
    +++ b/src/compiler/glsl/builtin_functions.cpp
    @@ -125,23 +125,21 @@ gs_only(const _mesa_glsl_parse_state *state)
      static bool
      v110(const _mesa_glsl_parse_state *state)
      {
         return !state->es_shader;
      }

      static bool
      v110_derivatives_only(const _mesa_glsl_parse_state *state)
      {
         return !state->es_shader &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      v120(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(120, 300);
      }

      static bool
      v130(const _mesa_glsl_parse_state *state)
    @@ -158,38 +156,34 @@ v130_desktop(const _mesa_glsl_parse_state *state)
      static bool
      v460_desktop(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(460, 0);
      }

      static bool
      v130_derivatives_only(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(130, 300) &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      v140_or_es3(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(140, 300);
      }

      static bool
      v400_derivatives_only(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(400, 0) &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      texture_rectangle(const _mesa_glsl_parse_state *state)
      {
         return state->ARB_texture_rectangle_enable;
      }

      static bool
      texture_external(const _mesa_glsl_parse_state *state)
    @@ -333,23 +327,21 @@ static bool
      gpu_shader4_tbo_integer(const _mesa_glsl_parse_state *state)
      {
         return gpu_shader4_tbo(state) &&
                state->ctx->Extensions.EXT_texture_integer;
      }

      static bool
      gpu_shader4_derivs_only(const _mesa_glsl_parse_state *state)
      {
         return state->EXT_gpu_shader4_enable &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      gpu_shader4_integer_derivs_only(const _mesa_glsl_parse_state *state)
      {
         return gpu_shader4_derivs_only(state) &&
                state->ctx->Extensions.EXT_texture_integer;
      }

      static bool
    @@ -435,37 +427,35 @@ fs_interpolate_at(const _mesa_glsl_parse_state
    *state)

      static bool
      texture_array_lod(const _mesa_glsl_parse_state *state)
      {
         return lod_exists_in_stage(state) &&
                (state->EXT_texture_array_enable ||
                 (state->EXT_gpu_shader4_enable &&
                  state->ctx->Extensions.EXT_texture_array));
      }

    -static bool
    -fs_texture_array(const _mesa_glsl_parse_state *state)
    -{
    -   return state->stage == MESA_SHADER_FRAGMENT &&
    -          (state->EXT_texture_array_enable ||
    -           (state->EXT_gpu_shader4_enable &&
    -            state->ctx->Extensions.EXT_texture_array));
    -}
    -
      static bool
      texture_array(const _mesa_glsl_parse_state *state)
      {
         return state->EXT_texture_array_enable ||
                (state->EXT_gpu_shader4_enable &&
                 state->ctx->Extensions.EXT_texture_array);
      }

    +static bool
    +texture_array_derivs_only(const _mesa_glsl_parse_state *state)
    +{
    +   return derivatives_only(state) &&
    +          texture_array(state);
    +}
    +
      static bool
      texture_multisample(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(150, 310) ||
                state->ARB_texture_multisample_enable;
      }

      static bool
      texture_multisample_array(const _mesa_glsl_parse_state *state)
      {
    @@ -485,42 +475,40 @@ static bool
      texture_samples_identical_array(const _mesa_glsl_parse_state *state)
      {
         return texture_multisample_array(state) &&
                state->EXT_shader_samples_identical_enable;
      }

      static bool
      derivatives_texture_cube_map_array(const _mesa_glsl_parse_state
    *state)
      {
         return state->has_texture_cube_map_array() &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      texture_cube_map_array(const _mesa_glsl_parse_state *state)
      {
         return state->has_texture_cube_map_array();
      }

      static bool
      texture_query_levels(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(430, 0) ||
                state->ARB_texture_query_levels_enable;
      }

      static bool
      texture_query_lod(const _mesa_glsl_parse_state *state)
      {
    -   return state->stage == MESA_SHADER_FRAGMENT &&
    +   return derivatives_only(state) &&
                (state->ARB_texture_query_lod_enable ||
                 state->EXT_texture_query_lod_enable);
      }

      static bool
      texture_gather_cube_map_array(const _mesa_glsl_parse_state *state)
      {
         return state->is_version(400, 320) ||
                state->ARB_texture_gather_enable ||
                state->ARB_gpu_shader5_enable ||
    @@ -549,54 +537,38 @@ static bool
      texture_gather_only_or_es31(const _mesa_glsl_parse_state *state)
      {
         return !state->is_version(400, 320) &&
                !state->ARB_gpu_shader5_enable &&
                !state->EXT_gpu_shader5_enable &&
                !state->OES_gpu_shader5_enable &&
                (state->ARB_texture_gather_enable ||
                 state->is_version(0, 310));
      }

    -/* Desktop GL or OES_standard_derivatives + fragment shader only */
    +/* Desktop GL or OES_standard_derivatives */
      static bool
    -fs_oes_derivatives(const _mesa_glsl_parse_state *state)
    +derivatives(const _mesa_glsl_parse_state *state)
      {
    -   return state->stage == MESA_SHADER_FRAGMENT &&
    +   return derivatives_only(state) &&
                (state->is_version(110, 300) ||
                 state->OES_standard_derivatives_enable ||
                 state->ctx->Const.AllowGLSLRelaxedES);
      }

      static bool
    -derivatives(const _mesa_glsl_parse_state *state)
    -{
    -   return fs_oes_derivatives(state) ||
    -          (state->stage == MESA_SHADER_COMPUTE &&
    -           state->NV_compute_shader_derivatives_enable);
    -}
    -
    -static bool
    -fs_derivative_control(const _mesa_glsl_parse_state *state)
    +derivative_control(const _mesa_glsl_parse_state *state)
      {
    -   return state->stage == MESA_SHADER_FRAGMENT &&
    +   return derivatives_only(state) &&
                (state->is_version(450, 0) ||
                 state->ARB_derivative_control_enable);
      }

    -static bool
    -derivative_control(const _mesa_glsl_parse_state *state)
    -{
    -   return fs_derivative_control(state) ||
    -      (state->stage == MESA_SHADER_COMPUTE &&
    -       state->NV_compute_shader_derivatives_enable);
    -}
    -
      static bool
      tex1d_lod(const _mesa_glsl_parse_state *state)
      {
         return !state->es_shader && lod_exists_in_stage(state);
      }

      /** True if sampler3D exists */
      static bool
      tex3d(const _mesa_glsl_parse_state *state)
      {
    @@ -605,23 +577,21 @@ tex3d(const _mesa_glsl_parse_state *state)
          */
         return !state->es_shader ||
                state->OES_texture_3D_enable ||
                state->language_version >= 300;
      }

      static bool
      derivatives_tex3d(const _mesa_glsl_parse_state *state)
      {
         return (!state->es_shader || state->OES_texture_3D_enable) &&
    -          (state->stage == MESA_SHADER_FRAGMENT ||
    -           (state->stage == MESA_SHADER_COMPUTE &&
    -            state->NV_compute_shader_derivatives_enable));
    +          derivatives_only(state);
      }

      static bool
      tex3d_lod(const _mesa_glsl_parse_state *state)
      {
         return tex3d(state) && lod_exists_in_stage(state);
      }

      static bool
      shader_atomic_counters(const _mesa_glsl_parse_state *state)
    @@ -3254,21 +3224,21 @@ builtin_builder::create_builtins()

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler1D_type,

    glsl_type::float_type),

_texture(ir_txb, v110_derivatives_only, glsl_type::vec4_type, glsl_type::sampler1D_type,

    glsl_type::float_type),

_texture(ir_tex, gpu_shader4_integer, glsl_type::ivec4_type, glsl_type::isampler1D_type,

    glsl_type::float_type),

_texture(ir_txb, gpu_shader4_integer_derivs_only, glsl_type::ivec4_type, glsl_type::isampler1D_type,

    glsl_type::float_type),

_texture(ir_tex, gpu_shader4_integer, glsl_type::uvec4_type, glsl_type::usampler1D_type,

    glsl_type::float_type),

_texture(ir_txb, gpu_shader4_integer_derivs_only, glsl_type::uvec4_type, glsl_type::usampler1D_type,

    glsl_type::float_type),
                      NULL);

         add_function("texture1DArray",

_texture(ir_tex, texture_array, glsl_type::vec4_type, glsl_type::sampler1DArray_type,

    glsl_type::vec2_type),

- _texture(ir_txb, fs_texture_array,glsl_type::vec4_type, glsl_type::sampler1DArray_type,

    glsl_type::vec2_type),
    +                _texture(ir_txb,
    texture_array_derivs_only,glsl_type::vec4_type,
    glsl_type::sampler1DArray_type, glsl_type::vec2_type),

_texture(ir_tex, gpu_shader4_array_integer, glsl_type::ivec4_type, glsl_type::isampler1DArray_type,

    glsl_type::vec2_type),
                      _texture(ir_txb,
    gpu_shader4_array_integer_derivs_only, glsl_type::ivec4_type,
    glsl_type::isampler1DArray_type, glsl_type::vec2_type),

_texture(ir_tex, gpu_shader4_array_integer, glsl_type::uvec4_type, glsl_type::usampler1DArray_type,

    glsl_type::vec2_type),
                      _texture(ir_txb,
    gpu_shader4_array_integer_derivs_only, glsl_type::uvec4_type,
    glsl_type::usampler1DArray_type, glsl_type::vec2_type),
                      NULL);

         add_function("texture1DProj",

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler1D_type,

    glsl_type::vec2_type, TEX_PROJECT),

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler1D_type,

    glsl_type::vec4_type, TEX_PROJECT),
                      _texture(ir_txb, v110_derivatives_only,
    glsl_type::vec4_type,  glsl_type::sampler1D_type,
    glsl_type::vec2_type, TEX_PROJECT),
    @@ -3309,21 +3279,21 @@ builtin_builder::create_builtins()

_texture(ir_txb, derivatives_only,glsl_type::vec4_type, glsl_type::sampler2D_type, glsl_type::vec2_type), _texture(ir_tex, gpu_shader4_integer, glsl_type::ivec4_type, glsl_type::isampler2D_type,

    glsl_type::vec2_type),
                      _texture(ir_txb, gpu_shader4_integer_derivs_only,
    glsl_type::ivec4_type,  glsl_type::isampler2D_type,
    glsl_type::vec2_type),

_texture(ir_tex, gpu_shader4_integer, glsl_type::uvec4_type, glsl_type::usampler2D_type,

    glsl_type::vec2_type),
                      _texture(ir_txb, gpu_shader4_integer_derivs_only,
    glsl_type::uvec4_type,  glsl_type::usampler2D_type,
    glsl_type::vec2_type),

_texture(ir_tex, texture_external,glsl_type::vec4_type, glsl_type::samplerExternalOES_type,

    glsl_type::vec2_type),
                      NULL);

         add_function("texture2DArray",

_texture(ir_tex, texture_array, glsl_type::vec4_type, glsl_type::sampler2DArray_type,

    glsl_type::vec3_type),

- _texture(ir_txb, fs_texture_array,glsl_type::vec4_type, glsl_type::sampler2DArray_type,

    glsl_type::vec3_type),
    +                _texture(ir_txb, texture_array_derivs_only,
    glsl_type::vec4_type, glsl_type::sampler2DArray_type,
    glsl_type::vec3_type),

_texture(ir_tex, gpu_shader4_array_integer, glsl_type::ivec4_type, glsl_type::isampler2DArray_type,

    glsl_type::vec3_type),
                      _texture(ir_txb,
    gpu_shader4_array_integer_derivs_only, glsl_type::ivec4_type,
    glsl_type::isampler2DArray_type, glsl_type::vec3_type),

_texture(ir_tex, gpu_shader4_array_integer, glsl_type::uvec4_type, glsl_type::usampler2DArray_type,

    glsl_type::vec3_type),
                      _texture(ir_txb,
    gpu_shader4_array_integer_derivs_only, glsl_type::uvec4_type,
    glsl_type::usampler2DArray_type, glsl_type::vec3_type),
                      NULL);

         add_function("texture2DProj",

_texture(ir_tex, always_available,glsl_type::vec4_type, glsl_type::sampler2D_type,

    glsl_type::vec3_type, TEX_PROJECT),

_texture(ir_tex, always_available,glsl_type::vec4_type, glsl_type::sampler2D_type,

    glsl_type::vec4_type, TEX_PROJECT),

_texture(ir_txb, derivatives_only,glsl_type::vec4_type, glsl_type::sampler2D_type,

    glsl_type::vec3_type, TEX_PROJECT),
    @@ -3421,41 +3391,41 @@ builtin_builder::create_builtins()
                      _texture(ir_tex, gpu_shader4_rect_integer,
    glsl_type::uvec4_type,  glsl_type::usampler2DRect_type,
    glsl_type::vec4_type, TEX_PROJECT),
                      NULL);

         add_function("shadow1D",

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler1DShadow_type,

    glsl_type::vec3_type),
                      _texture(ir_txb, v110_derivatives_only,
    glsl_type::vec4_type,  glsl_type::sampler1DShadow_type,
    glsl_type::vec3_type),
                      NULL);

         add_function("shadow1DArray",

_texture(ir_tex, texture_array,glsl_type::vec4_type, glsl_type::sampler1DArrayShadow_type,

    glsl_type::vec3_type),
    -                _texture(ir_txb, fs_texture_array,
    glsl_type::vec4_type,  glsl_type::sampler1DArrayShadow_type,
    glsl_type::vec3_type),
    +                _texture(ir_txb, texture_array_derivs_only,
    glsl_type::vec4_type,  glsl_type::sampler1DArrayShadow_type,
    glsl_type::vec3_type),
                      NULL);

         add_function("shadow2D",

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler2DShadow_type,

    glsl_type::vec3_type),
                      _texture(ir_txb, v110_derivatives_only,
    glsl_type::vec4_type,  glsl_type::sampler2DShadow_type,
    glsl_type::vec3_type),
                      NULL);

         add_function("shadow2DArray",

_texture(ir_tex, texture_array,glsl_type::vec4_type, glsl_type::sampler2DArrayShadow_type,

    glsl_type::vec4_type),
    -                _texture(ir_txb, fs_texture_array,
    glsl_type::vec4_type,  glsl_type::sampler2DArrayShadow_type,
    glsl_type::vec4_type),
    +                _texture(ir_txb, texture_array_derivs_only,
    glsl_type::vec4_type,  glsl_type::sampler2DArrayShadow_type,
    glsl_type::vec4_type),
                      NULL);

         add_function("shadow1DProj",

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler1DShadow_type,

    glsl_type::vec4_type, TEX_PROJECT),
                      _texture(ir_txb, v110_derivatives_only,
    glsl_type::vec4_type,  glsl_type::sampler1DShadow_type,
    glsl_type::vec4_type, TEX_PROJECT),
                      NULL);

         add_function("shadow2DArray",

_texture(ir_tex, texture_array,glsl_type::vec4_type, glsl_type::sampler2DArrayShadow_type,

    glsl_type::vec4_type),
    -                _texture(ir_txb, fs_texture_array,
    glsl_type::vec4_type,  glsl_type::sampler2DArrayShadow_type,
    glsl_type::vec4_type),
    +                _texture(ir_txb, texture_array_derivs_only,
    glsl_type::vec4_type,  glsl_type::sampler2DArrayShadow_type,
    glsl_type::vec4_type),
                      NULL);

         add_function("shadowCube",

_texture(ir_tex, gpu_shader4, glsl_type::vec4_type, glsl_type::samplerCubeShadow_type,

    glsl_type::vec4_type),
                      _texture(ir_txb, gpu_shader4_derivs_only,
    glsl_type::vec4_type, glsl_type::samplerCubeShadow_type,
    glsl_type::vec4_type),
                      NULL);

         add_function("shadow2DProj",

_texture(ir_tex, v110,glsl_type::vec4_type, glsl_type::sampler2DShadow_type,

    glsl_type::vec4_type, TEX_PROJECT),
                      _texture(ir_txb, v110_derivatives_only,
    glsl_type::vec4_type,  glsl_type::sampler2DShadow_type,
    glsl_type::vec4_type, TEX_PROJECT),

--2.17.1



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: fix and clean up NV_compute_shader_derivatives support

Reply via email to