On 26.04.2017 06:25, Timothy Arceri wrote:
On 24/04/17 20:35, Samuel Pitoiset wrote:
Yes, this is a bit hacky but we don't really have the choice.
Plain GLSL doesn't accept bindless samplers/images as l-values
while it's allowed when ARB_bindless_texture is enabled.

Are you sure we need this? Can't we just set all the bindless qualifier
flags to 0 when bindless is disable or something, and then just check if
any of the flags were set on the var?

Although I'm not sure the current change is so bad. Thoughts?

The problem with the alternative approach is that temporary ir_variable's are generated all over the place, and so you'd somehow need to decide in the constructor which bits to set. This seems very involved and fragile to me.

Better to acknowledge that yes, the spec simply changed its mind about whether samplers/images can be lvalues, and count our blessings that it doesn't really matter after ast_to_hir.

The places that end up passing NULL are places that are only sanity checks.

Patch is

Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>




The default NULL parameter is because we can't access the
_mesa_glsl_parse_state object in few places in the compiler.
One is_lvalue(NULL) call is for IR validation but other checks
happen elsewhere, should be safe.

Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
  src/compiler/glsl/ast_function.cpp | 2 +-
  src/compiler/glsl/ast_to_hir.cpp   | 2 +-
  src/compiler/glsl/ir.cpp           | 4 +++-
  src/compiler/glsl/ir.h             | 8 ++++----
  4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/compiler/glsl/ast_function.cpp
b/src/compiler/glsl/ast_function.cpp
index 1b90937ec8..00b930eb2c 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -283,7 +283,7 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
                               mode, formal->name,
                               actual->variable_referenced()->name);
              return false;
-         } else if (!actual->is_lvalue()) {
+         } else if (!actual->is_lvalue(state)) {
              _mesa_glsl_error(&loc, state,
                               "function parameter '%s %s' is not an
lvalue",
                               mode, formal->name);
diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 943c25a224..f90ae3d09a 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -971,7 +971,7 @@ do_assignment(exec_list *instructions, struct
_mesa_glsl_parse_state *state,
            * The restriction on arrays is lifted in GLSL 1.20 and
GLSL ES 3.00.
            */
           error_emitted = true;
-      } else if (!lhs->is_lvalue()) {
+      } else if (!lhs->is_lvalue(state)) {
           _mesa_glsl_error(& lhs_loc, state, "non-lvalue in
assignment");
           error_emitted = true;
        }
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 01f68a321c..b9c4452f83 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -24,6 +24,8 @@
  #include "main/core.h" /* for MAX2 */
  #include "ir.h"
  #include "compiler/glsl_types.h"
+#include "glsl_parser_extras.h"
+
    ir_rvalue::ir_rvalue(enum ir_node_type t)
     : ir_instruction(t)
@@ -1454,7 +1456,7 @@
ir_dereference_record::ir_dereference_record(ir_variable *var,
  }
    bool
-ir_dereference::is_lvalue() const
+ir_dereference::is_lvalue(const struct _mesa_glsl_parse_state *state)
const
  {
     ir_variable *var = this->variable_referenced();
  diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 584714ef82..bb900233ba 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -233,7 +233,7 @@ public:
       ir_rvalue *as_rvalue_to_saturate();
  -   virtual bool is_lvalue() const
+   virtual bool is_lvalue(const struct _mesa_glsl_parse_state *state
= NULL) const
     {
        return false;
     }
@@ -1934,9 +1934,9 @@ public:
     virtual bool equals(const ir_instruction *ir,
                         enum ir_node_type ignore = ir_type_unset) const;
  -   bool is_lvalue() const
+   bool is_lvalue(const struct _mesa_glsl_parse_state *state) const
     {
-      return val->is_lvalue() && !mask.has_duplicates;
+      return val->is_lvalue(state) && !mask.has_duplicates;
     }
       /**
@@ -1961,7 +1961,7 @@ class ir_dereference : public ir_rvalue {
  public:
     virtual ir_dereference *clone(void *mem_ctx, struct hash_table *)
const = 0;
  -   bool is_lvalue() const;
+   bool is_lvalue(const struct _mesa_glsl_parse_state *state) const;
       /**
      * Get the variable that is ultimately referenced by an r-value

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to