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 317b956..009659f 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 bfcf69f..9d2d10c 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 82e00a6..1aead1d 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 8ef7406..3731e7a 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 4d4bc0f..922dcd3 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 cdb634b..759924e 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 5fcc9ee..8481e2c 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 9f302d9..23365df 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 0024fd7..e370bc2 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 215c472..6ab34f3 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 638ffeb..e4628bc 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 67e4a36..cfd56ce 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 adb8720..3b84723 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 4d2a457..e450fec 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 a9a824f..8eb0dda 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;