<div dir="ltr"><div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 21, 2019 at 10:35 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi;<br>
<br>
On 2/11/19 6:46 PM, Oscar Blumberg wrote:<br>
> apply_implicit_conversion only converts and check base types but we<br>
> need actual type equality for function returns, otherwise you can<br>
> return a vec2 from a function declared as returning a float.<br>
<br>
Do you have some test shader that hits this condition? It seems to me <br>
that currently we will error out correctly if one tries to return vec2 <br>
from function declared as returning a float.<br>
<br></blockquote><div> </div><div>Yes, I believe it triggers in even the simplest case here, such as :</div><div>float f() { return vec2(1.0); }<br></div><div>anywhere in the shader.</div><div>Does it fail to reproduce on your end ?</div><div>(note that the declared glsl version must be recent enough that implicit conversion for function returns are enabled, I'm using 450 here).</div><div><br></div><div>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.</div><div>Because of that it only checks (and convert in place) base types without looking at element count for vector-like things.</div><div>We can still use it to perform the conversion but it requires an additional check, hence the patch.</div><div><br></div><div>That's my understanding of it anyway.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ---<br>
>   src/compiler/glsl/ast_to_hir.cpp | 3 ++-<br>
>   1 file changed, 2 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp<br>
> index 620153e6a34..6bf2910954f 100644<br>
> --- a/src/compiler/glsl/ast_to_hir.cpp<br>
> +++ b/src/compiler/glsl/ast_to_hir.cpp<br>
> @@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,<br>
>   <br>
>               if (state->has_420pack()) {<br>
>                  if (!apply_implicit_conversion(state->current_function->return_type,<br>
> -                                              ret, state)) {<br>
> +                                              ret, state)<br>
> +                   || (ret->type != state->current_function->return_type)) {<br>
>                     _mesa_glsl_error(& loc, state,<br>
>                                      "could not implicitly convert return value "<br>
>                                      "to %s, in function `%s'",<br>
> <br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a></blockquote></div></div></div>