Mesa (main): glsl: enable the use of the nir based varying linker
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon May 16 04:43:32 UTC 2022
Module: Mesa
Branch: main
Commit: 7647023f3bb5785f15476b64c08b3ed01c46c536
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=7647023f3bb5785f15476b64c08b3ed01c46c536
Author: Timothy Arceri <tarceri at itsqueeze.com>
Date: Tue Nov 23 12:44:07 2021 +1100
glsl: enable the use of the nir based varying linker
Here as well as calling the pass we need to switch the order of
some of the information gathering and optimisation calls. We also
need to create a custom callback for the dead variables removal
pass to clean up dead builtin varying in SSO programs without
causing piglit regressions.
shader-db results IRIS (BDW):
total instructions in shared programs: 17487900 -> 17477072 (-0.06%)
instructions in affected programs: 128682 -> 117854 (-8.41%)
helped: 587
HURT: 82
helped stats (abs) min: 1 max: 145 x̄: 18.82 x̃: 20
helped stats (rel) min: 0.21% max: 77.78% x̄: 17.41% x̃: 8.85%
HURT stats (abs) min: 1 max: 6 x̄: 2.68 x̃: 2
HURT stats (rel) min: 0.25% max: 9.76% x̄: 2.94% x̃: 2.16%
95% mean confidence interval for instructions value: -17.71 -14.66
95% mean confidence interval for instructions %-change: -16.40% -13.42%
Instructions are helped.
total cycles in shared programs: 857442520 -> 857170199 (-0.03%)
cycles in affected programs: 112252720 -> 111980399 (-0.24%)
helped: 13733
HURT: 13349
helped stats (abs) min: 1 max: 7293 x̄: 81.44 x̃: 10
helped stats (rel) min: <.01% max: 90.32% x̄: 3.30% x̃: 0.62%
HURT stats (abs) min: 1 max: 7424 x̄: 63.38 x̃: 8
HURT stats (rel) min: <.01% max: 192.23% x̄: 3.28% x̃: 0.54%
95% mean confidence interval for cycles value: -14.01 -6.10
95% mean confidence interval for cycles %-change: -0.17% 0.06%
Inconclusive result (%-change mean confidence interval includes 0).
total sends in shared programs: 971443 -> 970010 (-0.15%)
sends in affected programs: 4596 -> 3163 (-31.18%)
helped: 446
HURT: 39
helped stats (abs) min: 1 max: 6 x̄: 3.40 x̃: 4
helped stats (rel) min: 3.03% max: 85.71% x̄: 46.48% x̃: 50.00%
HURT stats (abs) min: 1 max: 3 x̄: 2.15 x̃: 2
HURT stats (rel) min: 6.67% max: 25.00% x̄: 15.16% x̃: 10.53%
95% mean confidence interval for sends value: -3.13 -2.78
95% mean confidence interval for sends %-change: -44.16% -38.88%
Sends are helped.
LOST: 235
GAINED: 262
Shader-db results radeonsi (RX580):
169505 shaders in 102144 tests
Totals:
SGPRS: 7698832 -> 7696552 (-0.03 %)
VGPRS: 5547296 -> 5545280 (-0.04 %)
Spilled SGPRs: 14795 -> 14773 (-0.15 %)
Spilled VGPRs: 3782 -> 3782 (0.00 %)
Private memory VGPRs: 1152 -> 1152 (0.00 %)
Scratch size: 3872 -> 3872 (0.00 %) dwords per thread
Code Size: 162946528 -> 162895264 (-0.03 %) bytes
Max Waves: 2449334 -> 2449736 (0.02 %)
Totals from affected shaders:
SGPRS: 215024 -> 212744 (-1.06 %)
VGPRS: 151976 -> 149960 (-1.33 %)
Spilled SGPRs: 162 -> 140 (-13.58 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 5249916 -> 5198652 (-0.98 %) bytes
Max Waves: 54588 -> 54990 (0.74 %)
Panfrost trace checksum is updated as per discussion in:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/6343
Some virpipe tess shader piglit tests are added as failures to CI
these failures are not a regression but an uncovered existing bug
exposed due to the linker no longer sorting internally facing
shader interfaces in alphabetical order. See details in:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/6481
Acked-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15731>
---
src/compiler/glsl/gl_nir_linker.c | 7 +++
src/compiler/glsl/gl_nir_linker.h | 1 +
src/compiler/glsl/glsl_to_nir.cpp | 9 ----
src/compiler/glsl/linker.cpp | 4 --
src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt | 9 ++++
src/mesa/state_tracker/st_glsl_to_ir.cpp | 2 -
src/mesa/state_tracker/st_glsl_to_nir.cpp | 64 ++++++++++++++++-------
src/panfrost/ci/traces-panfrost.yml | 2 +-
8 files changed, 63 insertions(+), 35 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_linker.c b/src/compiler/glsl/gl_nir_linker.c
index 8f6d4ee39b9..de1a7927680 100644
--- a/src/compiler/glsl/gl_nir_linker.c
+++ b/src/compiler/glsl/gl_nir_linker.c
@@ -781,8 +781,15 @@ check_image_resources(const struct gl_constants *consts,
bool
gl_nir_link_glsl(const struct gl_constants *consts,
const struct gl_extensions *exts,
+ gl_api api,
struct gl_shader_program *prog)
{
+ if (prog->NumShaders == 0)
+ return true;
+
+ if (!gl_nir_link_varyings(consts, exts, api, prog))
+ return false;
+
for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
struct gl_linked_shader *shader = prog->_LinkedShaders[i];
if (shader) {
diff --git a/src/compiler/glsl/gl_nir_linker.h b/src/compiler/glsl/gl_nir_linker.h
index df5d9fb8ba2..b07b18d45d7 100644
--- a/src/compiler/glsl/gl_nir_linker.h
+++ b/src/compiler/glsl/gl_nir_linker.h
@@ -58,6 +58,7 @@ bool gl_nir_link_spirv(const struct gl_constants *consts,
bool gl_nir_link_glsl(const struct gl_constants *consts,
const struct gl_extensions *exts,
+ gl_api api,
struct gl_shader_program *prog);
bool gl_nir_link_uniforms(const struct gl_constants *consts,
diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
index 5a920c54a76..601b87fc2ad 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -254,15 +254,6 @@ glsl_to_nir(const struct gl_constants *consts,
if (shader_prog->Label)
shader->info.label = ralloc_strdup(shader, shader_prog->Label);
- /* Check for transform feedback varyings specified via the API */
- shader->info.has_transform_feedback_varyings =
- shader_prog->TransformFeedback.NumVarying > 0;
-
- /* Check for transform feedback varyings specified in the Shader */
- if (shader_prog->last_vert_prog)
- shader->info.has_transform_feedback_varyings |=
- shader_prog->last_vert_prog->sh.LinkedTransformFeedback->NumVarying > 0;
-
if (shader->info.stage == MESA_SHADER_FRAGMENT) {
shader->info.fs.pixel_center_integer = sh->Program->info.fs.pixel_center_integer;
shader->info.fs.origin_upper_left = sh->Program->info.fs.origin_upper_left;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 160e3bbf593..f7de2866e27 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4100,10 +4100,6 @@ link_varyings_and_uniforms(unsigned first, unsigned last,
break;
}
- if (!link_varyings(prog, first, last, consts, exts,
- api, mem_ctx))
- return false;
-
if (!prog->data->LinkStatus)
return false;
diff --git a/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt b/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt
index 40ee5c0c868..b3f0eb0726d 100644
--- a/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt
+++ b/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt
@@ -536,11 +536,20 @@ spec at arb_shader_texture_lod@execution at arb_shader_texture_lod-texgradcube,Fail
spec at arb_shader_texture_lod@execution at glsl-fs-shadow2dgradarb-07,Fail
spec at arb_shader_texture_lod@execution at glsl-fs-shadow2dgradarb-08,Fail
spec at arb_shader_texture_lod@execution at glsl-fs-shadow2dgradarb-cumulative,Fail
+spec at arb_tessellation_shader@execution at barrier-patch,Fail
spec at arb_tessellation_shader@execution at double-array-vs-tcs-tes,Fail
spec at arb_tessellation_shader@execution at double-vs-tcs-tes,Fail
spec at arb_tessellation_shader@execution at dvec2-vs-tcs-tes,Fail
spec at arb_tessellation_shader@execution at dvec3-vs-tcs-tes,Fail
spec at arb_tessellation_shader@execution at gs-primitiveid-instanced,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-float-index-rd-after-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-float-index-wr-before-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec2-index-rd-after-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec2-index-wr-before-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec3-index-rd-after-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec3-index-wr-before-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec4-index-rd-after-barrier,Fail
+spec at arb_tessellation_shader@execution at variable-indexing@tcs-output-array-vec4-index-wr-before-barrier,Fail
# " intrinsic copy_deref (ssa_2, ssa_3) (dst_access=0, src_access=0)
# error: glsl_get_bare_type(dst->type) == glsl_get_bare_type(src->type) (../src/compiler/nir/nir_validate.c:643)"
diff --git a/src/mesa/state_tracker/st_glsl_to_ir.cpp b/src/mesa/state_tracker/st_glsl_to_ir.cpp
index b1424b0e00f..769226065dc 100644
--- a/src/mesa/state_tracker/st_glsl_to_ir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_ir.cpp
@@ -139,8 +139,6 @@ link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
validate_ir_tree(ir);
}
- build_program_resource_list(&ctx->Const, prog);
-
ret = st_link_nir(ctx, prog);
return ret;
diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 10df319593d..c379702d94a 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -293,6 +293,17 @@ shared_type_info(const struct glsl_type *type, unsigned *size, unsigned *align)
*align = comp_size * (length == 3 ? 4 : length);
}
+static bool
+st_can_remove_varying_before_linking(nir_variable *var, void *data)
+{
+ bool *is_sso = (bool *) data;
+ if (*is_sso) {
+ /* Allow the removal of unused builtins in SSO */
+ return var->data.location > -1 && var->data.location < VARYING_SLOT_VAR0;
+ } else
+ return true;
+}
+
/* First third of converting glsl_to_nir.. this leaves things in a pre-
* nir_lower_io state, so that shader variants can more easily insert/
* replace variables, etc.
@@ -342,15 +353,12 @@ st_nir_preprocess(struct st_context *st, struct gl_program *prog,
NIR_PASS_V(nir, st_nir_add_point_size);
}
- /* ES has strict SSO validation rules for shader IO matching so we can't
- * remove dead IO until the resource list has been built. Here we skip
- * removing them until later. This will potentially make the IO lowering
- * calls below do a little extra work but should otherwise have no impact.
- */
- if (!_mesa_is_gles(st->ctx) || !nir->info.separate_shader) {
- nir_variable_mode mask = nir_var_shader_in | nir_var_shader_out;
- nir_remove_dead_variables(nir, mask, NULL);
- }
+ struct nir_remove_dead_variables_options opts;
+ bool is_sso = nir->info.separate_shader;
+ opts.can_remove_var_data = &is_sso;
+ opts.can_remove_var = &st_can_remove_varying_before_linking;
+ nir_variable_mode mask = nir_var_shader_in | nir_var_shader_out;
+ nir_remove_dead_variables(nir, mask, &opts);
if (options->lower_all_io_to_temps ||
nir->info.stage == MESA_SHADER_VERTEX ||
@@ -737,15 +745,6 @@ st_link_nir(struct gl_context *ctx,
st_lower_patch_vertices_in(shader_program);
- /* Linking the stages in the opposite order (from fragment to vertex)
- * ensures that inter-shader outputs written to in an earlier stage
- * are eliminated if they are (transitively) not used in a later
- * stage.
- */
- for (int i = num_shaders - 2; i >= 0; i--) {
- st_nir_link_shaders(linked_shader[i]->Program->nir,
- linked_shader[i + 1]->Program->nir);
- }
/* Linking shaders also optimizes them. Separate shaders, compute shaders
* and shaders with a fixed-func VS or FS that don't need linking are
* optimized here.
@@ -754,15 +753,42 @@ st_link_nir(struct gl_context *ctx,
gl_nir_opts(linked_shader[0]->Program->nir);
if (shader_program->data->spirv) {
+ /* Linking the stages in the opposite order (from fragment to vertex)
+ * ensures that inter-shader outputs written to in an earlier stage
+ * are eliminated if they are (transitively) not used in a later
+ * stage.
+ */
+ for (int i = num_shaders - 2; i >= 0; i--) {
+ st_nir_link_shaders(linked_shader[i]->Program->nir,
+ linked_shader[i + 1]->Program->nir);
+ }
+
static const gl_nir_linker_options opts = {
true /*fill_parameters */
};
if (!gl_nir_link_spirv(&ctx->Const, shader_program, &opts))
return GL_FALSE;
} else {
- if (!gl_nir_link_glsl(&ctx->Const, &ctx->Extensions,
+ if (!gl_nir_link_glsl(&ctx->Const, &ctx->Extensions, ctx->API,
shader_program))
return GL_FALSE;
+
+ /* Linking the stages in the opposite order (from fragment to vertex)
+ * ensures that inter-shader outputs written to in an earlier stage
+ * are eliminated if they are (transitively) not used in a later
+ * stage.
+ */
+ for (int i = num_shaders - 2; i >= 0; i--) {
+ st_nir_link_shaders(linked_shader[i]->Program->nir,
+ linked_shader[i + 1]->Program->nir);
+ }
+
+ /* Tidy up any left overs from the linking process for single shaders.
+ * For example varying arrays that get packed may have dead elements that
+ * can be now be eliminated now that array access has been lowered.
+ */
+ if (num_shaders == 1)
+ gl_nir_opts(linked_shader[0]->Program->nir);
}
for (unsigned i = 0; i < num_shaders; i++) {
diff --git a/src/panfrost/ci/traces-panfrost.yml b/src/panfrost/ci/traces-panfrost.yml
index ddc8502c418..cb5c655cc63 100644
--- a/src/panfrost/ci/traces-panfrost.yml
+++ b/src/panfrost/ci/traces-panfrost.yml
@@ -78,7 +78,7 @@ traces:
- path: glmark2/bump:bump-render=height.trace
expectations:
- device: gl-panfrost-t860
- checksum: 2efca26d7eb85d826276b30f3265153b
+ checksum: 73cb74446b0aefcfcf7b41d80eaed016
- path: glmark2/bump:bump-render=high-poly.trace
expectations:
- device: gl-panfrost-t860
More information about the mesa-commit
mailing list