[Mesa-dev] [PATCH] i965: Drop support for the legacy SNORM -> Float equation.
Jason Ekstrand
jason at jlekstrand.net
Tue Dec 26 14:30:39 UTC 2017
Yes, please!
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Might be worth an ack from idr.
On December 26, 2017 00:57:30 Kenneth Graunke <kenneth at whitecape.org> wrote:
> Older OpenGL defines two equations for converting from signed-normalized
> to floating point data. These are:
>
> f = (2c + 1)/(2^b - 1) (equation 2.2)
> f = max{c/2^(b-1) - 1), -1.0} (equation 2.3)
>
> Both OpenGL 4.2+ and OpenGL ES 3.0+ mandate that equation 2.3 is to be
> used in all scenarios, and remove equation 2.2. DirectX uses equation
> 2.3 as well. Intel hardware only supports equation 2.3, so Gen7.5+
> systems that use the vertex fetcher hardware to do the conversions
> always get formula 2.3.
>
> This can make a big difference for 10-10-10-2 formats - the 2-bit value
> can represent 0 with equation 2.3, and cannot with equation 2.2.
>
> Ivybridge and older were using equation 2.2 for OpenGL, and 2.3 for ES.
> Now that Ivybridge supports OpenGL 4.2, this is wrong - we need to use
> the new rules, at least in core profile. That would leave Gen4-6 doing
> something different than all other hardware, which seems...lame.
>
> With context version promotion, applications that requested a pre-4.2
> context may get promoted to 4.2, and thus get the new rules. Zero cases
> have been reported of this being a problem. However, we've received a
> report that following the old rules breaks expectations. SuperTuxKart
> apparently renders the cars red when following equation 2.2, and works
> correctly when following equation 2.3:
>
> https://github.com/supertuxkart/stk-code/issues/2885#issuecomment-353858405
>
> So, this patch deletes the legacy equation 2.2 support entirely, making
> all hardware and APIs consistently use the new equation 2.3 rules.
>
> If we ever find an application that truly requires the old formula, then
> we'd likely want that application to work on modern hardware, too. We'd
> likely restore this support as a driconf option. Until then, drop it.
>
> This commit will regress Piglit's draw-vertices-2101010 test on
> pre-Haswell without the corresponding Piglit patch to accept either
> formula:
>
> draw-vertices-2101010: Accept either SNORM conversion formula.
> ---
> src/intel/blorp/blorp.c | 3 +--
> src/intel/compiler/brw_compiler.h | 1 -
> src/intel/compiler/brw_nir.c | 4 +--
> src/intel/compiler/brw_nir.h | 2 --
> src/intel/compiler/brw_nir_attribute_workarounds.c | 29 ++++++----------------
> src/intel/compiler/brw_vec4.cpp | 7 ++----
> src/intel/compiler/brw_vec4_vs.h | 5 +---
> src/intel/compiler/brw_vec4_vs_visitor.cpp | 6 ++---
> src/intel/vulkan/anv_pipeline.c | 2 +-
> src/mesa/drivers/dri/i965/brw_vs.c | 1 -
> 10 files changed, 15 insertions(+), 45 deletions(-)
>
> diff --git a/src/intel/blorp/blorp.c b/src/intel/blorp/blorp.c
> index 8a9d2fd3b97..e8a2c6135f5 100644
> --- a/src/intel/blorp/blorp.c
> +++ b/src/intel/blorp/blorp.c
> @@ -223,8 +223,7 @@ blorp_compile_vs(struct blorp_context *blorp, void
> *mem_ctx,
>
> const unsigned *program =
> brw_compile_vs(compiler, blorp->driver_ctx, mem_ctx,
> - &vs_key, vs_prog_data, nir,
> - false, -1, NULL);
> + &vs_key, vs_prog_data, nir, -1, NULL);
>
> return program;
> }
> diff --git a/src/intel/compiler/brw_compiler.h
> b/src/intel/compiler/brw_compiler.h
> index 28aed833245..0060c381c0d 100644
> --- a/src/intel/compiler/brw_compiler.h
> +++ b/src/intel/compiler/brw_compiler.h
> @@ -1123,7 +1123,6 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
> const struct brw_vs_prog_key *key,
> struct brw_vs_prog_data *prog_data,
> const struct nir_shader *shader,
> - bool use_legacy_snorm_formula,
> int shader_time_index,
> char **error_str);
>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 265c63efdda..dbddef0d04d 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -211,7 +211,6 @@ remap_patch_urb_offsets(nir_block *block, nir_builder *b,
>
> void
> brw_nir_lower_vs_inputs(nir_shader *nir,
> - bool use_legacy_snorm_formula,
> const uint8_t *vs_attrib_wa_flags)
> {
> /* Start with the location of the variable's base. */
> @@ -230,8 +229,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
>
> add_const_offset_to_base(nir, nir_var_shader_in);
>
> - brw_nir_apply_attribute_workarounds(nir, use_legacy_snorm_formula,
> - vs_attrib_wa_flags);
> + brw_nir_apply_attribute_workarounds(nir, vs_attrib_wa_flags);
>
> /* The last step is to remap VERT_ATTRIB_* to actual registers */
>
> diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
> index e9cb6f89948..809d4c338dc 100644
> --- a/src/intel/compiler/brw_nir.h
> +++ b/src/intel/compiler/brw_nir.h
> @@ -102,7 +102,6 @@ brw_nir_link_shaders(const struct brw_compiler *compiler,
> bool brw_nir_lower_cs_intrinsics(nir_shader *nir,
> unsigned dispatch_width);
> void brw_nir_lower_vs_inputs(nir_shader *nir,
> - bool use_legacy_snorm_formula,
> const uint8_t *vs_attrib_wa_flags);
> void brw_nir_lower_vue_inputs(nir_shader *nir,
> const struct brw_vue_map *vue_map);
> @@ -121,7 +120,6 @@ nir_shader *brw_postprocess_nir(nir_shader *nir,
> bool is_scalar);
>
> bool brw_nir_apply_attribute_workarounds(nir_shader *nir,
> - bool use_legacy_snorm_formula,
> const uint8_t *attrib_wa_flags);
>
> bool brw_nir_apply_trig_workarounds(nir_shader *nir);
> diff --git a/src/intel/compiler/brw_nir_attribute_workarounds.c
> b/src/intel/compiler/brw_nir_attribute_workarounds.c
> index c719371ddf1..55a3b3e4546 100644
> --- a/src/intel/compiler/brw_nir_attribute_workarounds.c
> +++ b/src/intel/compiler/brw_nir_attribute_workarounds.c
> @@ -33,7 +33,6 @@
> struct attr_wa_state {
> nir_builder builder;
> bool impl_progress;
> - bool use_legacy_snorm_formula;
> const uint8_t *wa_flags;
> };
>
> @@ -88,12 +87,15 @@ apply_attr_wa_block(nir_block *block, struct
> attr_wa_state *state)
> /* ES 3.0 has different rules for converting signed normalized
> * fixed-point numbers than desktop GL.
> */
> - if ((wa_flags & BRW_ATTRIB_WA_SIGN) &&
> - !state->use_legacy_snorm_formula) {
> + if (wa_flags & BRW_ATTRIB_WA_SIGN) {
> /* According to equation 2.2 of the ES 3.0 specification,
> * signed normalization conversion is done by:
> *
> * f = c / (2^(b-1)-1)
> + *
> + * OpenGL 4.2+ uses this equation as well. Since most contexts
> + * promote to the new higher version, and this is what Haswell+
> + * hardware does anyway, we just always use this formula.
> */
> nir_ssa_def *es3_normalize_factor =
> nir_imm_vec4(b, 1.0f / ((1 << 9) - 1), 1.0f / ((1 << 9) - 1),
> @@ -102,31 +104,16 @@ apply_attr_wa_block(nir_block *block, struct
> attr_wa_state *state)
> nir_fmul(b, nir_i2f32(b, val), es3_normalize_factor),
> nir_imm_float(b, -1.0f));
> } else {
> - /* The following equations are from the OpenGL 3.2 specification:
> + /* The following equation is from the OpenGL 3.2 specification:
> *
> * 2.1 unsigned normalization
> * f = c/(2^n-1)
> - *
> - * 2.2 signed normalization
> - * f = (2c+1)/(2^n-1)
> - *
> - * Both of these share a common divisor, which we handle by
> - * multiplying by 1 / (2^b - 1) for b = <10, 10, 10, 2>.
> */
> nir_ssa_def *normalize_factor =
> nir_imm_vec4(b, 1.0f / ((1 << 10) - 1), 1.0f / ((1 << 10) - 1),
> 1.0f / ((1 << 10) - 1), 1.0f / ((1 << 2) - 1));
>
> - if (wa_flags & BRW_ATTRIB_WA_SIGN) {
> - /* For signed normalization, the numerator is 2c+1. */
> - nir_ssa_def *two = nir_imm_float(b, 2.0f);
> - nir_ssa_def *one = nir_imm_float(b, 1.0f);
> - val = nir_fadd(b, nir_fmul(b, nir_i2f32(b, val), two), one);
> - } else {
> - /* For unsigned normalization, the numerator is just c. */
> - val = nir_u2f32(b, val);
> - }
> - val = nir_fmul(b, val, normalize_factor);
> + val = nir_fmul(b, nir_u2f32(b, val), normalize_factor);
> }
> }
>
> @@ -145,12 +132,10 @@ apply_attr_wa_block(nir_block *block, struct
> attr_wa_state *state)
>
> bool
> brw_nir_apply_attribute_workarounds(nir_shader *shader,
> - bool use_legacy_snorm_formula,
> const uint8_t *attrib_wa_flags)
> {
> bool progress = false;
> struct attr_wa_state state = {
> - .use_legacy_snorm_formula = use_legacy_snorm_formula,
> .wa_flags = attrib_wa_flags,
> };
>
> diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
> index 73c40ad6009..2ddfa4fb790 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -2744,7 +2744,6 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
> const struct brw_vs_prog_key *key,
> struct brw_vs_prog_data *prog_data,
> const nir_shader *src_shader,
> - bool use_legacy_snorm_formula,
> int shader_time_index,
> char **error_str)
> {
> @@ -2772,8 +2771,7 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
> prog_data->inputs_read = shader->info.inputs_read;
> prog_data->double_inputs_read = shader->info.double_inputs_read;
>
> - brw_nir_lower_vs_inputs(shader, use_legacy_snorm_formula,
> - key->gl_attrib_wa_flags);
> + brw_nir_lower_vs_inputs(shader, key->gl_attrib_wa_flags);
> brw_nir_lower_vue_outputs(shader, is_scalar);
> shader = brw_postprocess_nir(shader, compiler, is_scalar);
>
> @@ -2891,8 +2889,7 @@ brw_compile_vs(const struct brw_compiler *compiler,
> void *log_data,
> prog_data->base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT;
>
> vec4_vs_visitor v(compiler, log_data, key, prog_data,
> - shader, mem_ctx,
> - shader_time_index, use_legacy_snorm_formula);
> + shader, mem_ctx, shader_time_index);
> if (!v.run()) {
> if (error_str)
> *error_str = ralloc_strdup(mem_ctx, v.fail_msg);
> diff --git a/src/intel/compiler/brw_vec4_vs.h
> b/src/intel/compiler/brw_vec4_vs.h
> index b2a862fdbde..b62e03aa8d9 100644
> --- a/src/intel/compiler/brw_vec4_vs.h
> +++ b/src/intel/compiler/brw_vec4_vs.h
> @@ -37,8 +37,7 @@ public:
> struct brw_vs_prog_data *vs_prog_data,
> const nir_shader *shader,
> void *mem_ctx,
> - int shader_time_index,
> - bool use_legacy_snorm_formula);
> + int shader_time_index);
>
> protected:
> virtual void setup_payload();
> @@ -55,8 +54,6 @@ private:
>
> const struct brw_vs_prog_key *const key;
> struct brw_vs_prog_data * const vs_prog_data;
> -
> - bool use_legacy_snorm_formula;
> };
>
> } /* namespace brw */
> diff --git a/src/intel/compiler/brw_vec4_vs_visitor.cpp
> b/src/intel/compiler/brw_vec4_vs_visitor.cpp
> index 4d8ae23b0c7..8f15bc30a7c 100644
> --- a/src/intel/compiler/brw_vec4_vs_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_vs_visitor.cpp
> @@ -172,13 +172,11 @@ vec4_vs_visitor::vec4_vs_visitor(const struct
> brw_compiler *compiler,
> struct brw_vs_prog_data *vs_prog_data,
> const nir_shader *shader,
> void *mem_ctx,
> - int shader_time_index,
> - bool use_legacy_snorm_formula)
> + int shader_time_index)
> : vec4_visitor(compiler, log_data, &key->tex, &vs_prog_data->base, shader,
> mem_ctx, false /* no_spills */, shader_time_index),
> key(key),
> - vs_prog_data(vs_prog_data),
> - use_legacy_snorm_formula(use_legacy_snorm_formula)
> + vs_prog_data(vs_prog_data)
> {
> }
>
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
> index 407f2970199..4e66f8665fa 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -545,7 +545,7 @@ anv_pipeline_compile_vs(struct anv_pipeline *pipeline,
>
> const unsigned *shader_code =
> brw_compile_vs(compiler, NULL, mem_ctx, &key, &prog_data, nir,
> - false, -1, NULL);
> + -1, NULL);
> if (shader_code == NULL) {
> ralloc_free(mem_ctx);
> return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
> b/src/mesa/drivers/dri/i965/brw_vs.c
> index a56b256bc38..985d0bf04f0 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -221,7 +221,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
> char *error_str;
> program = brw_compile_vs(compiler, brw, mem_ctx, key, &prog_data,
> vp->program.nir,
> - !_mesa_is_gles3(&brw->ctx),
> st_index, &error_str);
> if (program == NULL) {
> if (!vp->program.is_arb_asm) {
> --
> 2.15.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list