Summary: | [glsl] Assertion failure when implicitly converting out parameters | ||
---|---|---|---|
Product: | Mesa | Reporter: | Chad Versace <chadversary> |
Component: | Drivers/DRI/i965 | Assignee: | Paul Berry <stereotype441> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | medium | ||
Version: | git | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
backtrace
testcase |
Description
Chad Versace
2011-07-28 16:59:57 UTC
Created attachment 49691 [details]
backtrace
Created attachment 49692 [details]
testcase
Confirmed that I can reproduce this bug. I'll start looking into it. 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; f(value); Into this: void f(int x); float value = 1.0; f(int(value)); 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; f(value); Would get transformed into this: void f(out int x); float value; int x_converted; f(x_converted); value = float(x_converted); And the "inout" case: void f(inout int x); float value = 1.0; f(value); Would get transformed into this: void f(inout int x); float value = 1.0; int x_converted = int(value); f(x_converted); 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. 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; f(x); U must have an implicit conversion to T. And in void f(out T x); U x; f(x); T must have an implicit conversion to U. And in void f(inout T x); U x; f(x); T and U most both implicitly convert to each other, which implies that T and U must be the same type. (In reply to comment #5) You're right. Thanks, Chad. This will save me from implementing some needlessly complex code. Ok, I've sent a patch series that fixes this bug to the Mesa-dev list for review. 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.