glsl: Hard-code noise to zero in builtin_functions.cpp
authorJason Ekstrand <jason@jlekstrand.net>
Mon, 20 Apr 2020 17:06:15 +0000 (12:06 -0500)
committerMarge Bot <eric+marge@anholt.net>
Tue, 21 Apr 2020 06:16:13 +0000 (06:16 +0000)
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@collabora.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4624>

16 files changed:
src/compiler/Makefile.sources
src/compiler/glsl/README
src/compiler/glsl/builtin_functions.cpp
src/compiler/glsl/glsl_to_nir.cpp
src/compiler/glsl/ir.cpp
src/compiler/glsl/ir_constant_expression.cpp
src/compiler/glsl/ir_expression_operation.py
src/compiler/glsl/ir_optimization.h
src/compiler/glsl/ir_validate.cpp
src/compiler/glsl/lower_noise.cpp [deleted file]
src/compiler/glsl/meson.build
src/compiler/glsl/test_optpass.cpp
src/mesa/drivers/dri/i965/brw_link.cpp
src/mesa/program/ir_to_mesa.cpp
src/mesa/state_tracker/st_glsl_to_ir.cpp
src/mesa/state_tracker/st_glsl_to_tgsi.cpp

index 317b95648baa771d5e3f06a52a6fc17a7fd93130..009659ff28af7f98df0b4bf744f9f924e8b6d69c 100644 (file)
@@ -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 \
index bfcf69f903af13237e82c9bb81c0f5ff4433a046..9d2d10c04a852b25ce777965ea5c4edd3a26bc2a 100644 (file)
@@ -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).
index 82e00a64ad9145579930d57b9db097216012af3b..1aead1d9e122143428fe455d2d888de3b95d271f 100644 (file)
@@ -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;
 }
 
index 8ef74066925266231c8223ff6cbffdfff6e4299f..3731e7a0b748b804fa85d543ba81247d16ae5ef9 100644 (file)
@@ -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,
index 4d4bc0fb957e32c88ffa8fb46549c57964fe2471..922dcd3c8198f9ee2553779e45c587ca7c59b757 100644 (file)
@@ -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:
index cdb634bd328021e64a28387a459f9d7f33b0a74b..759924ed9e1f1b8bb508a29b0a47481d40172a40 100644 (file)
@@ -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.
index 5fcc9ee9ab4da5f5af4ccac9d65d5c5b8b98b60f..8481e2ce640a765c1324a571c8c3f74d9dbf6601 100644 (file)
@@ -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
index 9f302d9fe51da1cfee3441ea14d17c3441fb8281..23365df10a38a71a4bf7ff2be1fe60b750de67a6 100644 (file)
@@ -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);
index 0024fd764d16d42f8e4afeb822e3f0b8d8d178d2..e370bc21b24ad93177fc6b600897b19dd252f098 100644 (file)
@@ -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 (file)
index 85f59b6..0000000
+++ /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@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;
-}
index 215c472ebaeadb21fd42ee86c0f0a7d631faabbf..6ab34f3014b50733b35aeadad23397de0063ff69 100644 (file)
@@ -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',
index 638ffeb2fe4904acb39471101b09e7307a8f625f..e4628bcc25bc8f3748f8b5e89472a9abe4944ae0 100644 (file)
@@ -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) {
index 67e4a36103fb5aada477b514228cfe24b70bd030..cfd56cef0df34201a5773d2bbbefce8adf15e766 100644 (file)
@@ -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);
index adb8720c622911a2d120f1d5cd57c8a09cf0f3d5..3b8472384ca9fcddeee4d3b5f7206ff079231cc8 100644 (file)
@@ -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.
          */
index 4d2a45728a9efb0fa568f354fa1616c27a47173e..e450fec6d99275e09de225fa5cd74ceb515d2562 100644 (file)
@@ -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);
       }
index a9a824f7bdaffc30da0e982b0eb517f3bdc8b5a1..8eb0dda9faeec860a9586989c157329d8929f858 100644 (file)
@@ -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;