<div dir="ltr">On 17 July 2013 18:24, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Rather than creating a new "binding" field in ir_variable, we reuse constant_value since the linker code for handling uniform initializers uses that. </blockquote><div> </div>I'm uncomfortable with this. It's already a bit irregular that we use constant_value for this purpose in handling uniform initializers (since the value of a uniform initializer can be overridden using the GL API, and hence isn't really constant), and we already have some ugly workaround code to deal with that irregularity (see ir_dereference_variable::constant_expression_value(), which has to have a special case to ignore constant_value when looking at a uniform). In point of fact, I'm worried that there may be bugs in opt_constant_variable.cpp, because it doesn't seem to go to any effort to avoid trying to treat uniforms with initializers as constants. <div> </div><div>Re-using constant_value for bindings seems even more dangerous, since now the types won't even match. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Since UBOs and samplers can't otherwise have initializers/constant values, there shouldn't be a conflict. Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> --- src/glsl/ast_to_hir.cpp | 7 +++++-- src/glsl/ir.h | 8 ++++++++ src/glsl/ir_clone.cpp | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 4a04b60..f438b6d 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2158,8 +2158,11 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, "explicit index requires explicit location\n"); } - if (qual->flags.q.explicit_binding) - validate_binding_qualifier(state, loc, var, qual); + if (qual->flags.q.explicit_binding && + validate_binding_qualifier(state, loc, var, qual)) { + var->explicit_binding = true; + var->constant_value = new(state) ir_constant(qual->binding); + } /* Does the declaration use the deprecated 'attribute' or 'varying' * keywords? diff --git a/src/glsl/ir.h b/src/glsl/ir.h index 1f0dc09..46bdb38 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -471,6 +471,14 @@ public: unsigned explicit_index:1; /** + * Was an initial binding explicitly set in the shader? + * + * If so, constant_value contains an integer ir_constant representing the + * initial binding point. + */ + unsigned explicit_binding:1; + + /** * Does this variable have an initializer? * * This is used by the linker to cross-validiate initializers of global diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp index 5b42935..535f5ce 100644 --- a/src/glsl/ir_clone.cpp +++ b/src/glsl/ir_clone.cpp @@ -55,6 +55,7 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const var->pixel_center_integer = this->pixel_center_integer; var->explicit_location = this->explicit_location; var->explicit_index = this->explicit_index; + var->explicit_binding = this->explicit_binding; var->has_initializer = this->has_initializer; var->depth_layout = this->depth_layout; -- 1.8.3.2 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>