Re: [Mesa-dev] [PATCH] glsl: Fix function return typechecking
Hi,
On Thu, Feb 21, 2019 at 10:35 AM Tapani Pälli
wrote:
> Hi;
>
> On 2/11/19 6:46 PM, Oscar Blumberg wrote:
> > apply_implicit_conversion only converts and check base types but we
> > need actual type equality for function returns, otherwise you can
> > return a vec2 from a function declared as returning a float.
>
> Do you have some test shader that hits this condition? It seems to me
> that currently we will error out correctly if one tries to return vec2
> from function declared as returning a float.
>
>
Yes, I believe it triggers in even the simplest case here, such as :
float f() { return vec2(1.0); }
anywhere in the shader.
Does it fail to reproduce on your end ?
(note that the declared glsl version must be recent enough that implicit
conversion for function returns are enabled, I'm using 450 here).
I believe most of the confusion comes from the name of the
apply_implicit_conversion function, since it is mainly used for arithmetic
operations for which, e.g., vec2+float is allowed.
Because of that it only checks (and convert in place) base types without
looking at element count for vector-like things.
We can still use it to perform the conversion but it requires an additional
check, hence the patch.
That's my understanding of it anyway.
> ---
> > src/compiler/glsl/ast_to_hir.cpp | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> > index 620153e6a34..6bf2910954f 100644
> > --- a/src/compiler/glsl/ast_to_hir.cpp
> > +++ b/src/compiler/glsl/ast_to_hir.cpp
> > @@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,
> >
> > if (state->has_420pack()) {
> > if
> (!apply_implicit_conversion(state->current_function->return_type,
> > - ret, state)) {
> > + ret, state)
> > + || (ret->type !=
> state->current_function->return_type)) {
> > _mesa_glsl_error(& loc, state,
> > "could not implicitly convert
> return value "
> > "to %s, in function `%s'",
> >
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix function return typechecking
On 2/21/19 11:35 AM, Tapani Pälli wrote:
Hi;
On 2/11/19 6:46 PM, Oscar Blumberg wrote:
apply_implicit_conversion only converts and check base types but we
need actual type equality for function returns, otherwise you can
return a vec2 from a function declared as returning a float.
Do you have some test shader that hits this condition? It seems to me
that currently we will error out correctly if one tries to return vec2
from function declared as returning a float.
Ah forget that, I can see it can happen with ARB_shading_language_420pack!
Reviewed-by: Tapani Pälli
---
src/compiler/glsl/ast_to_hir.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 620153e6a34..6bf2910954f 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,
if (state->has_420pack()) {
if
(!apply_implicit_conversion(state->current_function->return_type,
- ret, state)) {
+ ret, state)
+ || (ret->type !=
state->current_function->return_type)) {
_mesa_glsl_error(& loc, state,
"could not implicitly convert
return value "
"to %s, in function `%s'",
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix function return typechecking
Hi;
On 2/11/19 6:46 PM, Oscar Blumberg wrote:
apply_implicit_conversion only converts and check base types but we
need actual type equality for function returns, otherwise you can
return a vec2 from a function declared as returning a float.
Do you have some test shader that hits this condition? It seems to me
that currently we will error out correctly if one tries to return vec2
from function declared as returning a float.
---
src/compiler/glsl/ast_to_hir.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 620153e6a34..6bf2910954f 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,
if (state->has_420pack()) {
if
(!apply_implicit_conversion(state->current_function->return_type,
- ret, state)) {
+ ret, state)
+ || (ret->type != state->current_function->return_type)) {
_mesa_glsl_error(& loc, state,
"could not implicitly convert return value
"
"to %s, in function `%s'",
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Fix function return typechecking
apply_implicit_conversion only converts and check base types but we
need actual type equality for function returns, otherwise you can
return a vec2 from a function declared as returning a float.
---
src/compiler/glsl/ast_to_hir.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 620153e6a34..6bf2910954f 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,
if (state->has_420pack()) {
if
(!apply_implicit_conversion(state->current_function->return_type,
- ret, state)) {
+ ret, state)
+ || (ret->type != state->current_function->return_type)) {
_mesa_glsl_error(& loc, state,
"could not implicitly convert return value "
"to %s, in function `%s'",
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
