Commit graph

1282 commits

Author SHA1 Message Date
Ian Romanick
e458c3ddbb linker: Make linker_{error,warning} generally available
linker_warning is a new function.  It's identical to linker_error
except that it doesn't set LinkStatus=false and it prepends "warning: "
on messages instead of "error: ".

Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 379a32f42e)
2011-10-14 17:27:17 -07:00
Ian Romanick
b8a46f910d linker: Make linker_error set LinkStatus to false
Remove the other places that set LinkStatus to false since they all
immediately follow a call to linker_error.  The function linker_error
was previously known as linker_error_printf.  The name was changed
because it may seem surprising that a printf function will set an
error flag.

Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 586e741ac1)
2011-10-14 17:27:17 -07:00
Paul Berry
7d2ff4ae77 glsl: improve the accuracy of the asin() builtin function.
The previous formula for asin(x) was algebraically equivalent to:

sign(x)*(pi/2 - sqrt(1-|x|)*(A + B|x| + C|x|^2))

where A, B, and C were arbitrary constants determined by a curve fit.

This formula had a worst case absolute error of 0.00448, an unbounded
worst case relative error, and a discontinuity near x=0.

Changed the formula to:

sign(x)*(pi/2 - sqrt(1-|x|)*(pi/2 + (pi/4-1)|x| + A|x|^2 + B|x|^3))

where A and B are arbitrary constants determined by a curve fit.  This
has a worst case absolute error of 0.00039, a worst case relative
error of 0.000405, and no discontinuities.

I don't expect a significant performance degradation, since the extra
multiply-accumulate should be fast compared to the sqrt() computation.

Fixes piglit tests {vs,fs}-asin-float and {vs,fs}-atan-*
(cherry picked from commit d4c80f5f85)
2011-10-02 21:39:30 +02:00
Paul Berry
1bbf124ff8 glsl hierarchical visitor: Do not overwrite base_ir for parameter lists.
This patch fixes a bug in ir_hirearchical_visitor: when traversing an
exec_list representing the formal or actual parameters of a function,
it modified base_ir to point to each parameter in turn, rather than
leaving it as a pointer to the enclosing statement.  This was a
problem, since base_ir is used by visitor classes to locate the
statement containing the node being visited (usually so that
additional statements can be inserted before or after it).  Without
this fix, visitors might attempt to insert statements into parameter
lists.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit cc81eb09b9)
2011-10-02 19:57:57 +02:00
Eric Anholt
ca7560765c glsl: When assiging from a whole array, mark it as used.
Fixes piglit link-uniform-array-size.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 407a1001ae)
2011-10-02 19:57:57 +02:00
Eric Anholt
878d701da4 glsl: When assigning to a whole array, mark the array as accessed.
The vs-varying-array-mat2-col-row-wr test writes a mat2[3] constant to
a mat2[3] varying out array, and also statically accesses element 1 of
it on the VS and FS sides.  At link time it would get trimmed down to
just 2 elements, and then codegen of the VS would end up generating
assignments to the unallocated last entry of the array.  On the new
i965 VS backend, that happened to land on the vertex position.

Some issues remain in this test on softpipe, i965/old-vs and
i965/new-vs on visual inspection, but i965 is passing because only one
green pixel is probed, not the whole split green/red quad.
2011-10-02 19:57:56 +02:00
Paul Berry
c19b963ad6 glsl: Remove field array_lvalue from ir_variable.
The array_lvalue field was attempting to enforce the restriction that
whole arrays can't be used on the left-hand side of an assignment in
GLSL 1.10 or GLSL ES, and can't be used as out or inout parameters in
GLSL 1.10.

However, it was buggy (it didn't work properly for built-in arrays),
and it was clumsy (it unnecessarily kept track on a
variable-by-variable basis, and it didn't cover the GLSL ES case).

This patch removes the array_lvalue field completely in favor of
explicit checks in ast_parameter_declarator::hir() (this check is
added) and in do_assignment (this check was already present).

This causes a benign behavioral change: when the user attempts to pass
an array as an out or inout parameter of a function in GLSL 1.10, the
error is now flagged at the time the function definition is
encountered, rather than at the time of invocation.  Previously we
allowed such functions to be defined, and only flagged the error if
they were invoked.

Fixes Piglit tests
spec/glsl-1.10/compiler/qualifiers/fn-{out,inout}-array-prohibited*
and
spec/glsl-1.20/compiler/assignment-operators/assign-builtin-array-allowed.vert.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 00792e3586)
2011-10-02 19:57:08 +02:00
Eric Anholt
95185c7fe2 glsl: Clarify error message about whole-array assignment in GLSL 1.10.
Previously, it would produce:

    Failed to compile FS: 0:6(7): error: non-lvalue in assignment

and now it produces:

    Failed to compile FS: 0:5(7): error: whole array assignment is not
    allowed in GLSL 1.10 or GLSL ES 1.00.

Also, add spec quotation to the two places we have code for array
lvalues in GLSL 1.10.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 525cec98a5)
2011-10-02 19:56:53 +02:00
Paul Berry
e1221a8811 glsl: Rework oversize array check for gl_TexCoord.
The check now applies both when explicitly declaring the size of
gl_TexCoord and when implicitly setting the size of gl_TexCoord by
accessing it using integral constant expressions.

This is prep work for adding similar size checks to gl_ClipDistance.

Fixes piglit tests texcoord/implicit-access-max.{frag,vert}.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 93b9758d01)
2011-10-02 19:21:49 +02:00
Paul Berry
f732b5a999 glsl: Fix type error when lowering integer divisions
This patch fixes a bug when lowering an integer division:

  x/y

to a multiplication by a reciprocal:

  int(float(x)*reciprocal(float(y)))

If x was a plain int and y was an ivecN, the lowering pass
incorrectly assigned the type of the product to be float, when in fact
it should be vecN.  This caused mesa to abort with an IR validation
error.

Fixes piglit tests {fs,vs}-op-div-int-ivec{2,3,4}.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit af501e2b29)
2011-10-02 19:19:49 +02:00
Paul Berry
0129d5297b glsl: Perform implicit type conversions on function call out parameters.
When an out parameter undergoes an implicit type conversion, we need
to store it in a temporary, and then after the call completes, convert
the resulting value.  In other words, we convert code like the
following:

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

Into IR that's equivalent to this:

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

This transformation needs to happen during ast-to-IR convertion (as
opposed to, say, a lowering pass), because it is invalid IR for formal
and actual parameters to have types that don't match.

Fixes piglit tests
spec/glsl-1.20/compiler/qualifiers/out-conversion-int-to-float.vert and
spec/glsl-1.20/execution/qualifiers/vs-out-conversion-*.shader_test,
and bug 39651.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=39651

Reviewed-by: Chad Versace <chad@chad-versace.us>
(cherry picked from commit 67b5a3267d)
2011-10-02 19:19:08 +02:00
Paul Berry
27f00df2b7 glsl: Check array size is const before asserting that no IR was generated.
process_array_type() contains an assertion to verify that no IR
instructions are generated while processing the expression that
specifies the size of the array.  This assertion needs to happen
_after_ checking whether the expression is constant.  Otherwise we may
crash on an illegal shader rather than reporting an error.

Fixes piglit tests array-size-non-builtin-function.vert and
array-size-with-side-effect.vert.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit d4144a123b)
2011-10-02 19:17:42 +02:00
Paul Berry
8dcfe15a9a glsl: Constant-fold built-in functions before outputting IR
Rearranged the logic for converting the ast for a function call to
hir, so that we constant fold before emitting any IR.  Previously we
would emit some IR, and then only later detect whether we could
constant fold.  The unnecessary IR would usually get cleaned up by a
later optimization step, however in the case of a builtin function
being used to compute an array size, it was causing an assertion.

Fixes Piglit test array-size-constant-relational.vert.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38625
(cherry picked from commit 789ee6516b)
2011-10-02 19:16:47 +02:00
Paul Berry
2c0e00de23 glsl: Emit function signatures at toplevel, even for built-ins.
The ast-to-hir conversion needs to emit function signatures in two
circumstances: when a function declaration (or definition) is
encountered, and when a built-in function is encountered.

To avoid emitting a function signature in an illegal place (such as
inside a function), emit_function() checked whether we were inside a
function definition, and if so, emitted the signature before the
function definition.

However, this didn't cover the case of emitting function signatures
for built-in functions when those built-in functions are called from
inside the constant integer expression that specifies the length of a
global array.  This failed because when processing an array length, we
are emitting IR into a dummy exec_list (see process_array_type() in
ast_to_hir.cpp).  process_array_type() later checks (via an assertion)
that no instructions were emitted to the dummy exec_list, based on the
reasonable assumption that we shouldn't need to emit instructions to
calculate the value of a constant.

This patch changes emit_function() so that it emits function
signatures at toplevel in all cases.

This partially fixes bug 38625
(https://bugs.freedesktop.org/show_bug.cgi?id=38625).  The remainder
of the fix is in the patch that follows.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 0d81b0e184)
2011-10-02 19:03:22 +02:00
Paul Berry
1895de7a32 Revert "glsl: Skip processing the first function's body in do_dead_functions()."
opt_dead_functions contained a shortcut to skip processing the first
function's body, based on the assumption that IR functions are
topologically sorted, with callees always coming before their callers
(therefore the first function cannot contain any calls).

This assumption turns out not to be true in general.  For example, the
following code snippet gets translated to IR that violates this
assumption:

    void f();
    void g();
    void f() { g(); }
    void g() { ... }

In practice, the shortcut didn't cause bugs because of a coincidence
of the circumstances in which opt_dead_functions is called:

(a) we do inlining right before dead function elimination, and
    inlining (when successful) eliminates all calls.

(b) for user-defined functions, inlining is always successful, because
    previous optimization passes (during compilation) have reduced
    them to a form that is eligible for inlining.

(c) the function that appears first in the IR can't possibly call a
    built-in function, because built-in functions are always emitted
    before the function that calls them.

It seems unnecessarily fragile to have opt_dead_functions depend on
these coincidences.  And the next patch in this series will break (c).
So I'm reverting the shortcut.  The consequence will be a slight
increase in link time for complex shaders.

This reverts commit c75427f4c8.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 482338842d)
2011-10-02 19:02:30 +02:00
Paul Berry
7dc636dd77 glsl: improve the accuracy of the atan(x,y) builtin function.
The previous formula for atan(x,y) returned a value of +/- pi whenever
|x|<0.0001, and used a formula based on atan(y/x) otherwise.  This
broke in cases where both x and y were small (e.g. atan(1e-5, 1e-5)).

This patch modifies the formula so that it returns a value of +/- pi
whenever |x|<1e-8*|y|, and uses the formula based on atan(y/x)
otherwise.
(cherry picked from commit b1b4ea0b36)
2011-10-02 19:00:31 +02:00
Paul Berry
e42b822fec glsl: improve the accuracy of the radians() builtin function
The constant used in the radians() function didn't have enough
precision, causing a relative error of 1.676e-5, which is far worse
than the precision of 32-bit floats.  This patch reduces the relative
error to 1.14e-9, which is the best we can do in 32 bits.

Fixes piglit tests {fs,vs}-radians-{float,vec2,vec3,vec4}.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit fe33c886a7)
2011-10-02 18:50:28 +02:00
Paul Berry
0501cee136 glsl: Lower break instructions when necessary at the end of a loop.
Normally lower_jumps.cpp doesn't need to lower a break instruction
that occurs at the end of a loop, because all back-ends can produce
proper GPU instructions for a break instruction in this "canonical"
location.  However, if other break instructions within the loop are
already being lowered, then a break instruction at the end of the loop
needs to be lowered too, since after the optimization is complete a
new conditional break will be inserted at the end of the loop.

Without this patch, lower_jumps.cpp may require multiple passes in
order to lower all jumps.  This results in sub-optimal output because
lower_jumps.cpp produces a brand new set of temporary variables each
time it is run, and the redundant temporary variables are not
guaranteed to be eliminated by later optimization passes.

Fixes unit test test_lower_breaks_6.
(cherry picked from commit 067c9d7bd7)

Conflicts:

	src/glsl/lower_jumps.cpp
2011-10-02 18:48:27 +02:00
Paul Berry
38ae26b709 glsl: In lower_jumps.cpp, lower both branches of a conditional.
Previously, lower_jumps.cpp would break out of its loop after lowering
a jump instruction in just the then- or else-branch of a conditional,
and it would fail to lower a jump instruction occurring in the other
branch.

Without this patch, lower_jumps.cpp may require multiple passes in
order to lower all jumps.  This results in sub-optimal output because
lower_jumps.cpp produces a brand new set of temporary variables each
time it is run, and the redundant temporary variables are not
guaranteed to be eliminated by later optimization passes.

Fixes unit test test_lower_returns_4.
(cherry picked from commit e71b4ab8a6)
2011-10-02 18:45:10 +02:00
Paul Berry
de798938d4 glsl: Use foreach_list in lower_jumps.cpp
The visitor class in lower_jumps.cpp never removes or replaces the
instruction being visited, but it frequently alters or removes the
instructions that follow it.  Therefore, to make sure the altered IR
is visited, it needs to iterate through exec_lists using foreach_list
rather than visit_exec_list().

Without this patch, lower_jumps.cpp may require multiple passes in
order to lower all jumps.  This results in sub-optimal output because
lower_jumps.cpp produces a brand new set of temporary variables each
time it is run, and the redundant temporary variables are not
guaranteed to be eliminated by later optimization passes.

Also, certain invariants assumed by lower_jumps.cpp may fail to hold,
causing assertion failures.

Fixes unit tests test_lower_pulled_out_jump,
test_lower_unified_returns, test_lower_guarded_conditional_break,
test_lower_return_non_void_at_end_of_loop, and test_lower_returns_3.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 382cee91a4)
2011-10-02 18:44:50 +02:00
Paul Berry
acd2a03ffb glsl: lower unconditional returns and continues in loops.
Previously, lower_jumps.cpp would only lower return and continue
statements that appeared inside conditionals.  This patch makes it
lower unconditional returns and continue statements that occur inside
a loop.

Such unconditional flow control statements would be unlikely to be
explicitly coded by a reasonable user, however they might arise as a
result of other optimizations.

Without this patch, lower_jumps.cpp might not lower certain return and
continue statements, causing some backends to fail.

Fixes unit tests test_lower_return_void_at_end_of_loop and
test_remove_continue_at_end_of_loop.
(cherry picked from commit 03145ba655)

Conflicts:

	src/glsl/lower_jumps.cpp
2011-10-02 18:44:31 +02:00
Paul Berry
d1786cea1c glsl: Refactor logic for determining whether to lower return statements.
Previously, do_lower_jumps.cpp determined whether to lower return
statements in ir_lower_jumps_visitor::should_lower_jumps().  Moved
this logic to ir_lower_jumps_visitor::visit(ir_function_signature *),
so that it can be used in determining whether to lower a return
statement at the end of a function.
(cherry picked from commit dbaa2e627e)
2011-10-02 18:43:00 +02:00
Paul Berry
934c7a0661 glsl: Lower unconditional return statements.
Previously, lower_jumps.cpp only lowered return statements that
appeared inside of an if statement.

Without this patch, lower_jumps.cpp might not lower certain return
statements, causing some back-ends to fail (as in bug #36669).

Fixes unit test test_lower_returns_1.
(cherry picked from commit afc9a50fba)
2011-10-02 18:35:00 +02:00
Chia-I Wu
2cadae90c0 glsl: empty declarations should be valid
Unlike C++, empty declarations such as

  float;

should be valid.  The spec is not explicit about this actually.

Some apps that generate their shader sources may rely on this.  This was
noted when porting one of them to Linux from Windows.

Reviewed-by: Chad Versace <chad@chad-versace.us>

Note: this is a candidate for the 7.11 branch.
(cherry picked from commit 547212d963)
2011-10-02 17:59:26 +02:00
Chad Versace
6c032dd837 glsl: Fix conversions in array constructors
Array constructors obey narrower conversion rules than other constructors
[1] --- they use the implicit conversion rules [2] instead of the scalar
constructor conversions [3].  But process_array_constructor() was
incorrectly applying the broader rules.

[1] GLSL 1.50 spec, Section 5.4.4 Array Constructors, page 52 (58 of pdf)
[2] GLSL 1.50 spec, Section 4.1.10 Implicit Conversions, page 25 (31 of pdf)
[3] GLSL 1.50 spec, Section 5.4.1 Conversion, page 48 (54 of pdf)

To fix this, first check (with glsl_type::can_be_implicitly_converted_to)
if an implicit conversion is legal before performing the conversion.

Fixes:
piglit:spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bool-float.vert
piglit:spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bvec*-vec*.vert

Note: This is a candidate for the 7.10 and 7.11 branches.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Chad Versace <chad@chad-versace.us>
(cherry picked from commit a5ab9398e3)
2011-10-02 17:58:28 +02:00
Chad Versace
70c5be6c91 glsl: Remove ir_function.cpp:type_compare()
The function is no longer used and has been replaced by
glsl_type::can_implicitly_convert_to().

Note: This is a candidate for the 7.10 and 7.11 branches.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Chad Versace <chad@chad-versace.us>
(cherry picked from commit 6efe1a8495)
2011-10-02 17:00:59 +02:00
Chad Versace
3b92831fab glsl: Fix implicit conversions in non-constructor function calls
Context
-------
In ast_function_expression::hir(), parameter_lists_match() checks if the
function call's actual parameter list matches the signature's parameter
list, where the match may require implicit conversion of some arguments.
To check if an implicit conversion exists between individual arguments,
type_compare() is used.

Problems
--------
type_compare() allowed the following illegal implicit conversions:
    bool -> float
    bvecN -> vecN

    int -> uint
    ivecN -> uvecN

    uint -> int
    uvecN -> ivecN

Change
------
type_compare() is buggy, so replace it with glsl_type::can_be_implicitly_converted_to().
This comprises a rewrite of parameter_lists_match().

Fixes piglit:spec/glsl-1.20/compiler/built-in-functions/outerProduct-bvec*.vert

Note: This is a candidate for the 7.10 and 7.11 branches.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Chad Versace <chad@chad-versace.us>
(cherry picked from commit 8b3627fd7b)
2011-10-02 17:00:45 +02:00
Chad Versace
e6d07585f8 glsl: Add method glsl_type::can_implicitly_convert_to()
This method checks if a source type is identical to or can be implicitly
converted to a target type according to the GLSL 1.20 spec, Section 4.1.10
Implicit Conversions.

The following commits use the method for a bugfix:
    glsl: Fix implicit conversions in non-constructor function calls
    glsl: Fix implicit conversions in array constructors

Note: This is a candidate for the 7.10 and 7.11 branches.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Chad Versace <chad@chad-versace.us>
(cherry picked from commit 200e4972c1)
2011-10-02 16:57:32 +02:00
Kenneth Graunke
25861dc7f3 glsl: Avoid massive ralloc_strndup overhead in S-Expression parsing.
When parsing S-Expressions, we need to store nul-terminated strings for
Symbol nodes.  Prior to this patch, we called ralloc_strndup each time
we constructed a new s_symbol.  It turns out that this is obscenely
expensive.

Instead, copy the whole buffer before parsing and overwrite it to
contain \0 bytes at the appropriate locations.  Since atoms are
separated by whitespace, (), or ;, we can safely overwrite the character
after a Symbol.  While much of the buffer may be unused, copying the
whole buffer is simple and guaranteed to provide enough space.

Prior to this, running piglit-run.py -t glsl tests/quick.tests with GLSL
1.30 enabled took just over 10 minutes on my machine.  Now it takes 5.

NOTE: This is a candidate for stable release branches (because it will
      make running comparison tests so much less irritating.)

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 3875526926)
2011-10-02 16:50:39 +02:00
Eric Anholt
94e12df164 glsl: Allow ir_assignment() constructor to not specify condition.
We almost never want to specify a condition, and when we do we're
already thinking about it (because we're writing a lowering pass
generating the condition), so a default argument should make the code
more pleasant to read.

NOTE: This is a candidate for the 7.11 branch (we want to be able to
cherry-pick future code).
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit e617a53a74)
2011-10-02 16:31:16 +02:00
Ian Romanick
7d6c37b0c2 glsl: Treat ir_dereference_array of non-var as a constant for lowering
Previously the code would just look at deref->array->type to see if it
was a constant.  This isn't good enough because deref->array might be
another ir_dereference_array... of a constant.  As a result,
deref->array->type wouldn't be a constant, but
deref->variable_referenced() would return NULL.  The unchecked NULL
pointer would shortly lead to a segfault.

Instead just look at the return of deref->variable_referenced().  If
it's NULL, assume that either a constant or some other form of
anonymous temporary storage is being dereferenced.

This is a bit hinkey because most drivers treat constant arrays as
uniforms, but the lowering pass treats them as temporaries.  This
keeps the behavior of the old code, so this change isn't making things
worse.

Fixes i965 piglit:

    vs-temp-array-mat[234]-index-col-rd
    vs-temp-array-mat[234]-index-col-row-rd
    vs-uniform-array-mat[234]-index-col-rd
    vs-uniform-array-mat[234]-index-col-row-rd

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 156f85336f)
2011-07-28 11:47:45 -07:00
Ian Romanick
3a5a7cfd2a glsl: When lowering non-constant vector indexing, respect existing conditions
If the non-constant index was in the LHS of an assignment, any
existing condititon on that assignment would be lost.

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 601428d2bb)
2011-07-28 11:47:44 -07:00
Ian Romanick
aad12d15bb glsl: When lowering non-constant array indexing, respect existing conditions
If the non-constant index was in the LHS of an assignment, any
existing condititon on that assignment would be lost.

Fixes i965 piglit:

    fs-temp-array-mat[234]-col-row-wr
    fs-temp-array-mat[234]-index-col-row-wr
    fs-temp-array-mat[234]-index-col-wr
    fs-temp-array-mat[234]-index-row-wr
    vs-varying-array-mat[234]-index-col-wr

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 5f83dfe5b7)
2011-07-28 11:47:44 -07:00
Ian Romanick
977db7cc65 glsl: Rework lowering of non-constant array indexing
The previous implementation could easily get tricked if the LHS of an
assignment included a non-constant index that was "inside" another
dereference.  For example:

    mat4 m[2];
    m[0][i] = vec4(0.0);

Due to the way it tracked whether the array was being assigned, it
would think that the non-constant index was in an r-value.  The new
code fixes that by tracking l-values and r-values differently.  The
index is also replaced by cloning the IR and replacing the index
variable instead of the odd way it was done before.

v2: Apply some simplifications suggested by Eric Anholt.  Making
assignment_generator::rvalue be ir_dereference instead of ir_rvalue
simplified the code a bit.

Fixes i965 piglit fs-temp-array-mat[234]-index-wr and
vs-varying-array-mat[234]-index-wr.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34691
Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 1731ac3086)

To make bisects work, this also squashes in:

glsl: Correctly return progress from lower_variable_index_to_cond_assign

lower_variable_index_to_cond_assign runs until it can't make any more
progress.  It then returns the result of the last pass which will
always be false.  This caused the lowering loop in
_mesa_ir_link_shader to end before doing one last round of
lower_if_to_cond_assign.  This caused several if-statements (resulting
from lower_variable_index_to_cond_assign) to be left in the IR.

In addition to this change, lower_variable_index_to_cond_assign should
take a flag indicating whether or not it should even generate
if-statements.  This is easily controlled by
switch_generator::linear_sequence_max_length.  This would generate
much better code on architectures without any flow contol.

Fixes i915 piglit regressions glsl-texcoord-array and
glsl-fs-vec4-indexing-temp-src.

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit c1e591eed4)
2011-07-28 11:47:44 -07:00
Ian Romanick
d31c1c33ed glsl: Split out part of variable_index_to_cond_assign_visitor::needs_lowering
Other code will soon need to know if an array needs lowering based
exclusively on the storage mode.

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit d2296e784a)
2011-07-28 11:47:44 -07:00
Ian Romanick
3481a5a1e6 glsl: Move is_array_or_matrix outside visitor class
There's no reason for it to be there, and another class that may not
have access to the visitor will need it soon.

Reviewed-by: Eric Anholt <eric@anholt.net>
(cherry picked from commit 8d5f3cef79)
2011-07-28 11:47:44 -07:00
Ian Romanick
7c98381ed4 glsl: Reject shaders that contain static recursion
The GLSL 1.20 and later specs say:

    "Recursion is not allowed, not even statically. Static recursion is
    present if the static function call graph of the program contains
    cycles."

Recursion is detected and rejected both a compile-time and at
link-time.  The complie-time check happens to detect some cases that
may be removed by various optimization passes.  The spec doesn't seem
to allow this, but other vendors (e.g., NVIDIA) appear to only check
at link-time after all optimizations.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=33885
Reviewed-by: Paul Berry <stereotype441@gmail.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 02c5ae1b3f)

This also squashes in the following commit to make sure that bisects
in scons builds work:

glsl: Add ir_function_detect_recursion.cpp to SConscript.
(cherry picked from commit 76bccaff0c)
2011-07-23 01:55:25 -07:00
Ian Romanick
0ce6506d6e glsl: Make prototype_string publicly available
Also clarify the documentation for one of the parameters.

Reviewed-by: Paul Berry <stereotype441@gmail.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit 1ad3ba4ad9)
2011-07-23 01:53:59 -07:00
Brian Paul
25287008b6 glsl: silence warning in linker.cpp
(cherry picked from commit 4470ff2ebf)
2011-07-20 09:02:22 -07:00
Paul Berry
9aa8c02ae2 glsl: Rewrote _mesa_glsl_process_extension to use table-driven logic.
Instead of using a chain of manually maintained if/else blocks to
handle "#extension" directives, we now consult a table that specifies,
for each extension, the circumstances under which it is available, and
what flags in _mesa_glsl_parse_state need to be set in order to
activate it.

This makes it easier to add new GLSL extensions in the future, and
fixes the following bugs:

- Previously, _mesa_glsl_process_extension would sometimes set the
  "_enable" and "_warn" flags for an extension before checking whether
  the extension was supported by the driver; as a result, specifying
  "enable" behavior for an unsupported extension would sometimes cause
  front-end support for that extension to be switched on in spite of
  the fact that back-end support was not available, leading to strange
  failures, such as those in
  https://bugs.freedesktop.org/show_bug.cgi?id=38015.

- "#extension all: warn" and "#extension all: disable" had no effect.

Notes:

- All extensions are currently marked as unavailable in geometry
  shaders.  This should not have any adverse effects since geometry
  shaders aren't supported yet.  When we return to working on geometry
  shader support, we'll need to update the table for those extensions
  that are available in geometry shaders.

- Previous to this commit, if a shader mentioned
  ARB_shader_texture_lod, extension ARB_texture_rectangle would be
  automatically turned on in order to ensure that the types
  sampler2DRect and sampler2DRectShadow would be defined.  This was
  unnecessary, because (a) ARB_shader_texture_lod works perfectly well
  without those types provided that the builtin functions that
  reference them are not called, and (b) ARB_texture_rectangle is
  enabled by default in non-ES contexts anyway.  I eliminated this
  unnecessary behavior in order to make the behavior of all extensions
  consistent.

NOTE: This is a candidate for the 7.10 and 7.11 branches.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 3097715d41)
2011-07-20 08:15:32 -07:00
Paul Berry
b0ecde7f39 glsl: Changed extension enable bits to bools.
These were previously 1-bit-wide bitfields.  Changing them to bools
has a negligible performance impact, and allows them to be accessed by
offset as well as by direct structure access.

NOTE: This is a candidate for the 7.10 and 7.11 branches.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 9c4445de6e)
2011-07-20 08:15:21 -07:00
Kenneth Graunke
2826e3a000 glsl: Correctly handle function matching when there are multiple inexact matches
This is a squash cherry pick commit of:

    glsl: Find the "closest" signature when there are multiple matches.

    Previously, ir_function::matching_signature had a fatal bug: if a
    function had more than one non-exact match, it would simply return NULL.

    This occured, for example, when looking for max(uvec3, uvec3):
    - max(vec3, vec3)   -> score 1 (found first)
    - max(ivec3, ivec3) -> score 1 (found second...used to return NULL here)
    - max(uvec3, uvec3) -> score 0 (exact match...the right answer)

    This did not occur for max(ivec3, ivec3) since the second match found
    was an exact match.

    The new behavior is to return a match with the lowest score.  If there
    is an exact match, that will be returned.  Otherwise, a match with the
    least number of implicit conversions is chosen.

    Fixes piglit tests max-uvec3.vert and glsl-inexact-overloads.shader_test.

    NOTE: This is a candidate for the 7.10 and 7.11 branches.

    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>
    (cherry picked from commit 60eb63a855)

    glsl: Suppress warning from matching_signature change.

    gcc isn't smart enough to see that we only look at matched_score after
    we've initialized it (because match != NULL happens at the same time)
    (cherry picked from commit b043409adf)

    glsl: Reject ambiguous function calls (multiple inexact matches).

    According to the GLSL 1.20 specification, "it is a semantic error if
    there are multiple ways to apply [implicit] conversions [...] such that
    the call can be made to match multiple signatures."

    Fixes a regression caused by 60eb63a855,
    which implemented the wrong policy of finding a "closest" match.
    However, this is not a revert, since the original code failed to
    continue looking for an exact match once it found two inexact matches.

    It's OK to have multiple inexact matches if there's also an exact match.

    NOTE: This is a candidate for the 7.10 and 7.11 branches.

    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38971
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    (cherry picked from commit 7304909d65)
2011-07-19 16:39:56 -07:00
Paul Berry
f80ae99cbd glsl: Ensure that sampler declarations are always uniform or "in" parameters.
This brings us into compliance with page 17 (page 22 of the PDF) of
the GLSL 1.20 spec:

    "[Sampler types] can only be declared as function parameters or
    uniform variables (see Section 4.3.5 "Uniform"). ... [Samplers]
    cannot be used as out or inout function parameters."

The spec isn't explicit about whether this rule applies to
structs/arrays containing shaders, but the intent seems to be to
ensure that it can always be determined at compile time which sampler
is being used in each texture lookup.  So to avoid creating a
loophole, the rule needs to apply to structs/arrays containing shaders
as well.

Fixes piglit tests spec/glsl-1.10/compiler/samplers/*.frag, and fixes
bug 38987.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38987
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
(cherry picked from commit f07221056e)
2011-07-19 16:39:56 -07:00
Paul Berry
ae11fb02dd glsl: Move type_contains_sampler() into glsl_type for later reuse.
The new location, as a member function of glsl_type, is more
consistent with queries like is_sampler(), is_boolean(), is_float(),
etc.  Placing the function inside glsl_type also makes it available to
any code that uses glsl_types.
(cherry picked from commit ddc1c96390)
2011-07-19 16:39:56 -07:00
Ian Romanick
a90b88f354 linker: Only over-ride built-ins when a prototype has been seen
The GLSL spec says:

    "If a built-in function is redeclared in a shader (i.e., a
    prototype is visible) before a call to it, then the linker will
    only attempt to resolve that call within the set of shaders that
    are linked with it."

This patch enforces this behavior.  When a function call is processed
a flag is set in the ir_call to indicate whether the previously seen
prototype is the built-in or not.  At link time a call will only bind
to an instance of a function that matches the "want built-in" setting
in the ir_call.

This has the odd side effect that first call to abs() in the shader
below will call the built-in and the second will not:

float foo(float x) { return abs(x); }
float abs(float x) { return -x; }
float bar(float x) { return abs(x); }

This seems insane, but it matches what the spec says.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=31744
(cherry picked from commit 66f4ac988d)
2011-07-19 16:39:56 -07:00
Eric Anholt
5a7d1c9710 glsl: Fix make clean for dricore.
(cherry picked from commit abbbd14dd4)
2011-07-13 13:08:23 -07:00
Ian Romanick
530c68d616 glsl: Fix depth unbalancing problem in if-statement flattening
Previously, if max_depth were 1, the following code would see the
first if-statement (correctly) not get flattened, but the second
if-statement would (incorrectly) get flattened:

void main()
{
    if (a)
        gl_Position = vec4(0);

    if (b)
        gl_Position = vec4(1);
}

This is because the visit_leave(ir_if*) method would not decrement the
depth before returning on the first if-statement.

NOTE: This is a candidate for the 7.10 and 7.11 branches.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
(cherry picked from commit d2c6cef18a)
2011-07-08 16:02:20 -07:00
Brian Paul
c4da12e74f glsl: use casts to silence warning
(cherry picked from commit 7eb7d67d50)
2011-07-08 08:03:56 -06:00
Paul Berry
98af042079 glsl: permit explicit locations on fragment shader outputs, not inputs
From the OpenGL docs for GL_ARB_explicit_attrib_location:

    This extension provides a method to pre-assign attribute locations to
    named vertex shader inputs and color numbers to named fragment shader
    outputs.

This was accidentally implemented for fragment shader inputs.  This
patch fixes it to apply to fragment shader outputs.

Fixes piglit tests
spec/ARB_explicit_attrib_location/1.{10,20}/compiler/layout-{01,03,06,07,08,09,10}.frag

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

NOTE: This is a candidate for the 7.10 and 7.11 branches.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38624
(cherry picked from commit b078aad8ab)
2011-07-07 14:23:08 -07:00
Ian Romanick
127bd9d5b6 linker: Assign locations for fragment shader output
Fixes an assertion failure in the piglib out-01.frag
ARB_explicit_attrib_location test.  The locations set via the layout
qualifier in fragment shader were not being applied to the shader
outputs.  As a result all of these variables still had a location of
-1 set.

This may need some more work for pre-3.0 contexts.  The problem is
dealing with generic outputs that lack a layout qualifier.  There is
no way for the application to specify a location
(glBindFragDataLocation is not supported) or query the location
assigned by the linker (glGetFragDataLocation is not supported).

NOTE: This is a candidate for the 7.10 and 7.11 branches.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38624
Reviewed-by: Eric Anholt <eric@anholt.net>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Vinson Lee <vlee@vmware.com>
(cherry picked from commit d32d4f780f)
2011-07-06 17:06:29 -07:00