On 28.04.2017 03:15, Timothy Arceri wrote:
On 26/04/17 18:27, Samuel Pitoiset wrote:
On 04/26/2017 10:05 AM, Nicolai Hähnle wrote:
On 24.04.2017 12:36, Samuel Pitoiset wrote:
Because the variable declaration holds more information than
the dereference. Note that an image is considered bindless either
if it has been declared in the default uniform block with the
bindless_sampler layout qualifier, or when its storage is not
uniform because this is not allowed without ARB_bindless_texture.
It seems unfortunate that we have to do this. Can you explain what
goes wrong without this change?
We lost the variable and all information contained in ir_variable for
images.
Can you give an example of a shader where this becomes an issue? And why?
Samuel provided me with one, but the basic problem is actually quite
simple. Consider this (using GLSL rather than the IR, but I hope you get
the drift):
coherent image2D img = (some expression);
out = imageLoad(img, ...);
Tree grafting will convert that to
out = imageLoad((some expression), ...);
But now the coherent qualifier is gone.
In a way, GLSL is not very well specified here: the qualifiers should
really be part of the type, at least in the same way that C/C++ have the
cv-qualifiers, but they aren't.
There is another problem, which is the "mere" implementation problem
that st_glsl_to_tgsi is only set up to handle sampler/image parameters
to intrinsics that are direct dereferences. visit_image_intrinsics has a
cast from ir_rvalue to ir_dereference, which is simply incorrect when
that parameter is an expression.
The easy answer is to just not do tree grafting for samplers and images.
The cleaner answer is to disable tree grafting only when any of the
data.image_* qualifiers are set on the variable to be grafted, and to
fix st_glsl_to_tgsi so that it also handles expressions as sampler/image
parameters to intrinsics.
Cheers,
Nicolai
Thanks,
Nicolai
Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
src/compiler/glsl/opt_tree_grafting.cpp | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/compiler/glsl/opt_tree_grafting.cpp
b/src/compiler/glsl/opt_tree_grafting.cpp
index 28b6e1856e..d4a1ec5675 100644
--- a/src/compiler/glsl/opt_tree_grafting.cpp
+++ b/src/compiler/glsl/opt_tree_grafting.cpp
@@ -371,6 +371,15 @@ tree_grafting_basic_block(ir_instruction
*bb_first,
if (lhs_var->data.precise)
continue;
+ if (lhs_var->type->is_image() &&
+ (lhs_var->data.bindless || lhs_var->data.mode !=
ir_var_uniform)) {
+ /* Disable tree grafting optimization for bindless image
types because
+ * the variable declaration holds more information than the
+ * dereference.
+ */
+ continue;
+ }
+
ir_variable_refcount_entry *entry =
info->refs->get_variable_entry(lhs_var);
if (!entry->declaration ||
_______________________________________________
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
