Bug 39651 - [glsl] Assertion failure when implicitly converting out parameters
Summary: [glsl] Assertion failure when implicitly converting out parameters
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Paul Berry
QA Contact:
Depends on:
Reported: 2011-07-28 16:59 UTC by Chad Versace
Modified: 2011-08-16 10:59 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

backtrace (1.85 KB, application/octet-stream)
2011-07-28 17:00 UTC, Chad Versace
testcase (183 bytes, application/octet-stream)
2011-07-28 17:01 UTC, Chad Versace

Description Chad Versace 2011-07-28 16:59:57 UTC
Test case and backtrace is attached.

Assertion failure
glslparsertest: opt_constant_variable.cpp:140: virtual ir_visitor_status ir_constant_variable_visitor::visit_enter(ir_call*): Assertion `var' failed. 

Tested against
commit e4fdc95277bd323d8945e20635d3a1702a2e695d
Author: Brian Paul <brianp@vmware.com>
Date:   Thu Jul 28 09:51:30 2011 -0600
Comment 1 Chad Versace 2011-07-28 17:00:53 UTC
Created attachment 49691 [details]
Comment 2 Chad Versace 2011-07-28 17:01:28 UTC
Created attachment 49692 [details]
Comment 3 Paul Berry 2011-08-01 16:25:39 UTC
Confirmed that I can reproduce this bug.  I'll start looking into it.
Comment 4 Paul Berry 2011-08-01 17:23:06 UTC
What's happening is, when match_function_by_name() sees a parameter that needs conversion, it replaces the actual parameter with an expression that converts it to the type required by the function signature, effectively transforming this:

void f(int x);
float value = 1.0;

Into this:

void f(int x);
float value = 1.0;

For in parameters, this is correct.  But for out and inout parameters, it creates invalid IR, since the expression passed to an out or inout parameter must be an lvalue.  Unfortunately, ir_validate.cpp doesn't detect that this is invalid, so the assertion doesn't happen until later, when an optimization pass tries to see what variable is referred to in the lvalue, and fails.

What match_function_by_name() ought to do is create a temporary of the correct type, and then do the appropriate type conversion after the call (and possibly also before, in the case of an inout parameter).

So the "out" case:

void f(out int x);
float value;

Would get transformed into this:

void f(out int x);
float value;
int x_converted;
value = float(x_converted);

And the "inout" case:

void f(inout int x);
float value = 1.0;

Would get transformed into this:

void f(inout int x);
float value = 1.0;
int x_converted = int(value);
value = float(x_converted);

I'll work on implementing this.  I'll also add logic to ir_validate.cpp so that it raises a fuss if we try to pass non-lvalues to out and inout parameters.  And I'll add some piglit tests to make sure the conversions are done correctly.
Comment 5 Chad Versace 2011-08-01 19:56:33 UTC
Before tackling this bug, take a second to review the implicit conversion rules. They're in the GLSL 1.30 spec, Section 4.1.10 Implicit Conversions.

The only implicit conversions are:
  int, uint -> float
  ivecN, uvecN -> vecN

According to those rules, in
  void f(in T x);
  U x;
U must have an implicit conversion to T.

And in
  void f(out T x);
  U x;
T must have an implicit conversion to U.

And in
  void f(inout T x);
  U x;
T and U most both implicitly convert to each other, which implies that T and U must be the same type.
Comment 6 Paul Berry 2011-08-02 13:30:27 UTC
(In reply to comment #5)

You're right.  Thanks, Chad.  This will save me from implementing some needlessly complex code.
Comment 7 Paul Berry 2011-08-02 17:41:43 UTC
Ok, I've sent a patch series that fixes this bug to the Mesa-dev list for review.
Comment 8 Paul Berry 2011-08-16 10:59:42 UTC
Fixed in commit 67b5a3267d639c31d3ac4073be877ffb0f5637d3

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.