Mesa (master): glsl: Hard-code noise to zero in builtin_functions.cpp
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Apr 21 06:33:06 UTC 2020
Module: Mesa
Branch: master
Commit: 4386c06770508d86eaa51839871767887f903d1a
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=4386c06770508d86eaa51839871767887f903d1a
Author: Jason Ekstrand <jason at jlekstrand.net>
Date: Mon Apr 20 12:06:15 2020 -0500
glsl: Hard-code noise to zero in builtin_functions.cpp
Version 4.4 of the GLSL spec changed the definition of noise*() to
always return zero and earlier versions of the spec allowed zero as a
valid implementation.
All drivers, as far as I can tell, unconditionally call lower_noise()
today which turns ir_unop_noise into zero. We've got a 10-year-old
comment in there saying "In the future, ir_unop_noise may be replaced by
a call to a function that implements noise." Well, it's the future now
and we've not yet gotten around to that. In the mean time, the GLSL
spec has made doing so illegal.
To make things worse, we then pretend to handle the opcode in
glsl_to_nir, ir_to_mesa, and st_glsl_to_tgsi even though it should never
get there given the lowering. The lowering in st_glsl_to_tgsi defines
noise*() to be 0.5 which is an illegal implementation of the noise
functions according to pre-4.4 specs. We also have opcodes for this in
NIR which are never used because, again, we always call lower_noise().
Let's just kill the whole opcode and make builtin_builder.cpp build a
bunch of functions that just return zero.
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Eric Anholt <eric at anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4624>
---
src/compiler/Makefile.sources | 1 -
src/compiler/glsl/README | 2 +-
src/compiler/glsl/builtin_functions.cpp | 97 +++++++---------------------
src/compiler/glsl/glsl_to_nir.cpp | 42 ------------
src/compiler/glsl/ir.cpp | 3 -
src/compiler/glsl/ir_constant_expression.cpp | 12 +++-
src/compiler/glsl/ir_expression_operation.py | 2 -
src/compiler/glsl/ir_optimization.h | 1 -
src/compiler/glsl/ir_validate.cpp | 4 --
src/compiler/glsl/lower_noise.cpp | 71 --------------------
src/compiler/glsl/meson.build | 1 -
src/compiler/glsl/test_optpass.cpp | 2 -
src/mesa/drivers/dri/i965/brw_link.cpp | 1 -
src/mesa/program/ir_to_mesa.cpp | 11 ----
src/mesa/state_tracker/st_glsl_to_ir.cpp | 1 -
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 11 ----
16 files changed, 33 insertions(+), 229 deletions(-)
diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 317b95648ba..009659ff28a 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -108,7 +108,6 @@ LIBGLSL_FILES = \
glsl/lower_int64.cpp \
glsl/lower_jumps.cpp \
glsl/lower_mat_op_to_vec.cpp \
- glsl/lower_noise.cpp \
glsl/lower_offset_array.cpp \
glsl/lower_packed_varyings.cpp \
glsl/lower_named_interface_blocks.cpp \
diff --git a/src/compiler/glsl/README b/src/compiler/glsl/README
index bfcf69f903a..9d2d10c04a8 100644
--- a/src/compiler/glsl/README
+++ b/src/compiler/glsl/README
@@ -220,7 +220,7 @@ Q: What is the file naming convention in this directory?
Initially, there really wasn't one. We have since adopted one:
- Files that implement code lowering passes should be named lower_*
- (e.g., lower_noise.cpp).
+ (e.g., lower_builtins.cpp).
- Files that implement optimization passes should be named opt_*.
- Files that implement a class that is used throught the code should
take the name of that class (e.g., ir_hierarchical_visitor.cpp).
diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
index 82e00a64ad9..1aead1d9e12 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -6673,103 +6673,52 @@ builtin_builder::_fwidthFine(const glsl_type *type)
ir_function_signature *
builtin_builder::_noise1(const glsl_type *type)
{
- return unop(v110, ir_unop_noise, glsl_type::float_type, type);
+ /* From the GLSL 4.60 specification:
+ *
+ * "The noise functions noise1, noise2, noise3, and noise4 have been
+ * deprecated starting with version 4.4 of GLSL. When not generating
+ * SPIR-V they are defined to return the value 0.0 or a vector whose
+ * components are all 0.0. When generating SPIR-V the noise functions
+ * are not declared and may not be used."
+ *
+ * In earlier versions of the GLSL specification attempt to define some
+ * sort of statistical noise function. However, the function's
+ * characteristics have always been such that always returning 0 is
+ * valid and Mesa has always returned 0 for noise on most drivers.
+ */
+ ir_variable *p = in_var(type, "p");
+ MAKE_SIG(glsl_type::float_type, v110, 1, p);
+ body.emit(ret(imm(glsl_type::float_type, ir_constant_data())));
+ return sig;
}
ir_function_signature *
builtin_builder::_noise2(const glsl_type *type)
{
+ /* See builtin_builder::_noise1 */
ir_variable *p = in_var(type, "p");
MAKE_SIG(glsl_type::vec2_type, v110, 1, p);
-
- ir_constant_data b_offset;
- b_offset.f[0] = 601.0f;
- b_offset.f[1] = 313.0f;
- b_offset.f[2] = 29.0f;
- b_offset.f[3] = 277.0f;
-
- ir_variable *a = body.make_temp(glsl_type::float_type, "a");
- ir_variable *b = body.make_temp(glsl_type::float_type, "b");
- ir_variable *t = body.make_temp(glsl_type::vec2_type, "t");
- body.emit(assign(a, expr(ir_unop_noise, p)));
- body.emit(assign(b, expr(ir_unop_noise, add(p, imm(type, b_offset)))));
- body.emit(assign(t, a, WRITEMASK_X));
- body.emit(assign(t, b, WRITEMASK_Y));
- body.emit(ret(t));
-
+ body.emit(ret(imm(glsl_type::vec2_type, ir_constant_data())));
return sig;
}
ir_function_signature *
builtin_builder::_noise3(const glsl_type *type)
{
+ /* See builtin_builder::_noise1 */
ir_variable *p = in_var(type, "p");
MAKE_SIG(glsl_type::vec3_type, v110, 1, p);
-
- ir_constant_data b_offset;
- b_offset.f[0] = 601.0f;
- b_offset.f[1] = 313.0f;
- b_offset.f[2] = 29.0f;
- b_offset.f[3] = 277.0f;
-
- ir_constant_data c_offset;
- c_offset.f[0] = 1559.0f;
- c_offset.f[1] = 113.0f;
- c_offset.f[2] = 1861.0f;
- c_offset.f[3] = 797.0f;
-
- ir_variable *a = body.make_temp(glsl_type::float_type, "a");
- ir_variable *b = body.make_temp(glsl_type::float_type, "b");
- ir_variable *c = body.make_temp(glsl_type::float_type, "c");
- ir_variable *t = body.make_temp(glsl_type::vec3_type, "t");
- body.emit(assign(a, expr(ir_unop_noise, p)));
- body.emit(assign(b, expr(ir_unop_noise, add(p, imm(type, b_offset)))));
- body.emit(assign(c, expr(ir_unop_noise, add(p, imm(type, c_offset)))));
- body.emit(assign(t, a, WRITEMASK_X));
- body.emit(assign(t, b, WRITEMASK_Y));
- body.emit(assign(t, c, WRITEMASK_Z));
- body.emit(ret(t));
-
+ body.emit(ret(imm(glsl_type::vec3_type, ir_constant_data())));
return sig;
}
ir_function_signature *
builtin_builder::_noise4(const glsl_type *type)
{
+ /* See builtin_builder::_noise1 */
ir_variable *p = in_var(type, "p");
MAKE_SIG(glsl_type::vec4_type, v110, 1, p);
-
- ir_variable *_p = body.make_temp(type, "_p");
-
- ir_constant_data p_offset;
- p_offset.f[0] = 1559.0f;
- p_offset.f[1] = 113.0f;
- p_offset.f[2] = 1861.0f;
- p_offset.f[3] = 797.0f;
-
- body.emit(assign(_p, add(p, imm(type, p_offset))));
-
- ir_constant_data offset;
- offset.f[0] = 601.0f;
- offset.f[1] = 313.0f;
- offset.f[2] = 29.0f;
- offset.f[3] = 277.0f;
-
- ir_variable *a = body.make_temp(glsl_type::float_type, "a");
- ir_variable *b = body.make_temp(glsl_type::float_type, "b");
- ir_variable *c = body.make_temp(glsl_type::float_type, "c");
- ir_variable *d = body.make_temp(glsl_type::float_type, "d");
- ir_variable *t = body.make_temp(glsl_type::vec4_type, "t");
- body.emit(assign(a, expr(ir_unop_noise, p)));
- body.emit(assign(b, expr(ir_unop_noise, add(p, imm(type, offset)))));
- body.emit(assign(c, expr(ir_unop_noise, _p)));
- body.emit(assign(d, expr(ir_unop_noise, add(_p, imm(type, offset)))));
- body.emit(assign(t, a, WRITEMASK_X));
- body.emit(assign(t, b, WRITEMASK_Y));
- body.emit(assign(t, c, WRITEMASK_Z));
- body.emit(assign(t, d, WRITEMASK_W));
- body.emit(ret(t));
-
+ body.emit(ret(imm(glsl_type::vec4_type, ir_constant_data())));
return sig;
}
diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
index 8ef74066925..3731e7a0b74 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -2047,48 +2047,6 @@ nir_visitor::visit(ir_expression *ir)
result = nir_find_lsb(&b, srcs[0]);
break;
- case ir_unop_noise:
- switch (ir->type->vector_elements) {
- case 1:
- switch (ir->operands[0]->type->vector_elements) {
- case 1: result = nir_fnoise1_1(&b, srcs[0]); break;
- case 2: result = nir_fnoise1_2(&b, srcs[0]); break;
- case 3: result = nir_fnoise1_3(&b, srcs[0]); break;
- case 4: result = nir_fnoise1_4(&b, srcs[0]); break;
- default: unreachable("not reached");
- }
- break;
- case 2:
- switch (ir->operands[0]->type->vector_elements) {
- case 1: result = nir_fnoise2_1(&b, srcs[0]); break;
- case 2: result = nir_fnoise2_2(&b, srcs[0]); break;
- case 3: result = nir_fnoise2_3(&b, srcs[0]); break;
- case 4: result = nir_fnoise2_4(&b, srcs[0]); break;
- default: unreachable("not reached");
- }
- break;
- case 3:
- switch (ir->operands[0]->type->vector_elements) {
- case 1: result = nir_fnoise3_1(&b, srcs[0]); break;
- case 2: result = nir_fnoise3_2(&b, srcs[0]); break;
- case 3: result = nir_fnoise3_3(&b, srcs[0]); break;
- case 4: result = nir_fnoise3_4(&b, srcs[0]); break;
- default: unreachable("not reached");
- }
- break;
- case 4:
- switch (ir->operands[0]->type->vector_elements) {
- case 1: result = nir_fnoise4_1(&b, srcs[0]); break;
- case 2: result = nir_fnoise4_2(&b, srcs[0]); break;
- case 3: result = nir_fnoise4_3(&b, srcs[0]); break;
- case 4: result = nir_fnoise4_4(&b, srcs[0]); break;
- default: unreachable("not reached");
- }
- break;
- default:
- unreachable("not reached");
- }
- break;
case ir_unop_get_buffer_size: {
nir_intrinsic_instr *load = nir_intrinsic_instr_create(
this->shader,
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 4d4bc0fb957..922dcd3c819 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -345,9 +345,6 @@ ir_expression::ir_expression(int op, ir_rvalue *op0)
this->type = glsl_type::get_instance(GLSL_TYPE_UINT64,
op0->type->vector_elements, 1);
break;
- case ir_unop_noise:
- this->type = glsl_type::float_type;
- break;
case ir_unop_unpack_double_2x32:
case ir_unop_unpack_uint_2x32:
diff --git a/src/compiler/glsl/ir_constant_expression.cpp b/src/compiler/glsl/ir_constant_expression.cpp
index cdb634bd328..759924ed9e1 100644
--- a/src/compiler/glsl/ir_constant_expression.cpp
+++ b/src/compiler/glsl/ir_constant_expression.cpp
@@ -1083,10 +1083,16 @@ ir_function_signature::constant_expression_value(void *mem_ctx,
/*
* Of the builtin functions, only the texture lookups and the noise
- * ones must not be used in constant expressions. They all include
- * specific opcodes so they don't need to be special-cased at this
- * point.
+ * ones must not be used in constant expressions. Texture instructions
+ * include special ir_texture opcodes which can't be constant-folded (see
+ * ir_texture::constant_expression_value). Noise functions, however, we
+ * have to special case here.
*/
+ if (strcmp(this->function_name(), "noise1") == 0 ||
+ strcmp(this->function_name(), "noise2") == 0 ||
+ strcmp(this->function_name(), "noise3") == 0 ||
+ strcmp(this->function_name(), "noise4") == 0)
+ return NULL;
/* Initialize the table of dereferencable names with the function
* parameters. Verify their const-ness on the way.
diff --git a/src/compiler/glsl/ir_expression_operation.py b/src/compiler/glsl/ir_expression_operation.py
index 5fcc9ee9ab4..8481e2ce640 100644
--- a/src/compiler/glsl/ir_expression_operation.py
+++ b/src/compiler/glsl/ir_expression_operation.py
@@ -567,8 +567,6 @@ ir_expression_operation = [
operation("frexp_sig", 1),
operation("frexp_exp", 1),
- operation("noise", 1),
-
operation("subroutine_to_int", 1),
# Interpolate fs input at centroid
diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h
index 9f302d9fe51..23365df10a3 100644
--- a/src/compiler/glsl/ir_optimization.h
+++ b/src/compiler/glsl/ir_optimization.h
@@ -135,7 +135,6 @@ bool do_vec_index_to_swizzle(exec_list *instructions);
bool lower_discard(exec_list *instructions);
void lower_discard_flow(exec_list *instructions);
bool lower_instructions(exec_list *instructions, unsigned what_to_lower);
-bool lower_noise(exec_list *instructions);
bool lower_variable_index_to_cond_assign(gl_shader_stage stage,
exec_list *instructions, bool lower_input, bool lower_output,
bool lower_temp, bool lower_uniform);
diff --git a/src/compiler/glsl/ir_validate.cpp b/src/compiler/glsl/ir_validate.cpp
index 0024fd764d1..e370bc21b24 100644
--- a/src/compiler/glsl/ir_validate.cpp
+++ b/src/compiler/glsl/ir_validate.cpp
@@ -557,10 +557,6 @@ ir_validate::visit_leave(ir_expression *ir)
assert(ir->type->base_type == GLSL_TYPE_UINT);
break;
- case ir_unop_noise:
- /* XXX what can we assert here? */
- break;
-
case ir_unop_interpolate_at_centroid:
assert(ir->operands[0]->type == ir->type);
assert(ir->operands[0]->type->is_float_16_32());
diff --git a/src/compiler/glsl/lower_noise.cpp b/src/compiler/glsl/lower_noise.cpp
deleted file mode 100644
index 85f59b675e0..00000000000
--- a/src/compiler/glsl/lower_noise.cpp
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- * Copyright © 2010 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
- * DEALINGS IN THE SOFTWARE.
- */
-
-/**
- * \file lower_noise.cpp
- * IR lower pass to remove noise opcodes.
- *
- * \author Ian Romanick <ian.d.romanick at intel.com>
- */
-
-#include "ir.h"
-#include "ir_rvalue_visitor.h"
-
-class lower_noise_visitor : public ir_rvalue_visitor {
-public:
- lower_noise_visitor() : progress(false)
- {
- /* empty */
- }
-
- void handle_rvalue(ir_rvalue **rvalue)
- {
- if (!*rvalue)
- return;
-
- ir_expression *expr = (*rvalue)->as_expression();
- if (!expr)
- return;
-
- /* In the future, ir_unop_noise may be replaced by a call to a function
- * that implements noise. No hardware has a noise instruction.
- */
- if (expr->operation == ir_unop_noise) {
- *rvalue = ir_constant::zero(ralloc_parent(expr), expr->type);
- this->progress = true;
- }
- }
-
- bool progress;
-};
-
-
-bool
-lower_noise(exec_list *instructions)
-{
- lower_noise_visitor v;
-
- visit_list_elements(&v, instructions);
-
- return v.progress;
-}
diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build
index 215c472ebae..6ab34f3014b 100644
--- a/src/compiler/glsl/meson.build
+++ b/src/compiler/glsl/meson.build
@@ -158,7 +158,6 @@ files_libglsl = files(
'lower_int64.cpp',
'lower_jumps.cpp',
'lower_mat_op_to_vec.cpp',
- 'lower_noise.cpp',
'lower_offset_array.cpp',
'lower_packed_varyings.cpp',
'lower_named_interface_blocks.cpp',
diff --git a/src/compiler/glsl/test_optpass.cpp b/src/compiler/glsl/test_optpass.cpp
index 638ffeb2fe4..e4628bcc25b 100644
--- a/src/compiler/glsl/test_optpass.cpp
+++ b/src/compiler/glsl/test_optpass.cpp
@@ -116,8 +116,6 @@ do_optimization(struct exec_list *ir, const char *optimization,
} else if (sscanf(optimization, "lower_instructions ( %d ) ",
&int_0) == 1) {
return lower_instructions(ir, int_0);
- } else if (strcmp(optimization, "lower_noise") == 0) {
- return lower_noise(ir);
} else if (sscanf(optimization, "lower_variable_index_to_cond_assign "
"( %d , %d , %d , %d ) ", &int_0, &int_1, &int_2,
&int_3) == 4) {
diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index 67e4a36103f..cfd56cef0df 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -135,7 +135,6 @@ process_glsl_ir(struct brw_context *brw,
do_vec_index_to_cond_assign(shader->ir);
lower_vector_insert(shader->ir, true);
lower_offset_arrays(shader->ir);
- lower_noise(shader->ir);
lower_quadop_vector(shader->ir, false);
validate_ir_tree(shader->ir);
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index adb8720c622..3b8472384ca 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -1030,15 +1030,6 @@ ir_to_mesa_visitor::visit(ir_expression *ir)
inst->saturate = true;
break;
}
- case ir_unop_noise: {
- const enum prog_opcode opcode =
- prog_opcode(OPCODE_NOISE1
- + (ir->operands[0]->type->vector_elements) - 1);
- assert((opcode >= OPCODE_NOISE1) && (opcode <= OPCODE_NOISE4));
-
- emit(ir, opcode, result_dst, op[0]);
- break;
- }
case ir_binop_add:
emit(ir, OPCODE_ADD, result_dst, op[0], op[1]);
@@ -3022,8 +3013,6 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
progress = lower_if_to_cond_assign((gl_shader_stage)i, ir,
options->MaxIfDepth) || progress;
- progress = lower_noise(ir) || progress;
-
/* If there are forms of indirect addressing that the driver
* cannot handle, perform the lowering pass.
*/
diff --git a/src/mesa/state_tracker/st_glsl_to_ir.cpp b/src/mesa/state_tracker/st_glsl_to_ir.cpp
index 4d2a45728a9..e450fec6d99 100644
--- a/src/mesa/state_tracker/st_glsl_to_ir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_ir.cpp
@@ -159,7 +159,6 @@ st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
do_vec_index_to_cond_assign(ir);
lower_vector_insert(ir, true);
lower_quadop_vector(ir, false);
- lower_noise(ir);
if (options->MaxIfDepth == 0) {
lower_discard(ir);
}
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index a9a824f7bda..8eb0dda9fae 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -1572,17 +1572,6 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* ir, st_src_reg *op)
emit_asm(ir, TGSI_OPCODE_DFRACEXP, undef_dst, result_dst, op[0]);
break;
- case ir_unop_noise: {
- /* At some point, a motivated person could add a better
- * implementation of noise. Currently not even the nvidia
- * binary drivers do anything more than this. In any case, the
- * place to do this is in the GL state tracker, not the poor
- * driver.
- */
- emit_asm(ir, TGSI_OPCODE_MOV, result_dst, st_src_reg_for_float(0.5));
- break;
- }
-
case ir_binop_add:
emit_asm(ir, TGSI_OPCODE_ADD, result_dst, op[0], op[1]);
break;
More information about the mesa-commit
mailing list