[Mesa-dev] [PATCH] i965: Drop support for the legacy SNORM -> Float equation.
Kenneth Graunke
kenneth at whitecape.org
Tue Dec 26 06:56:50 UTC 2017
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
More information about the mesa-dev
mailing list