Avoid unrollong loops that are either nested loops or
where the loop body times the unroll count is huge.
The change is far from being perfect but it extends the
loop unrolling decision heuristic by some additional
safeguard. In particular this cuts down compilation of
a shader precomputing atmospheric scattering integral
tables containing two nesting levels in a loop from
something way beyond some minutes (I never waited for
it to finish) to some fractions of a second.
This fixes piglit tests glsl-fs-unroll-explosion and
glsl-vs-unroll-explosion on r600g.
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Mathias Fröhlich <Mathias.Froehlich@web.de>
The build was broken by the line below, added in commit 4f82fed4.
s_expression.cpp:26: #include <limits>
Mesa's half of the fix is to add 'external/astl/include' to the include
path. The other half of the fix requires implementing
numeric_limits<float>::infinity() in astl, for which I have patches
submitted upstream for review.
Signed-off-by: Chad Versace <chad.versace@linux.intel.com>
The error message I chose matches gcc's error. Fixes piglit
switch-case-duplicated.vert.
NOTE: This is a candidate for the 8.0 branch.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Otherwise, the upcoming error messages said the location was 0:0(0).
NOTE: This is a candidate for the 8.0 branch.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
It's not quite spelled out in the spec text, but the grammar indicates
that only constant values are allowed as switch() case labels (and
only constant values make sense, anyway).
Fixes piglit glsl-1.30/compiler/switch-statement/switch-case-uniform-int.vert.
NOTE: This is a candidate for the 8.0 branch.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
This stuffs them all in a struct for sanity. Fixes piglit
glsl-1.30/execution/switch/fs-uniform-nested.
NOTE: This is a candidate for the 8.0 branch.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
In commit 6ecee54a9a a call to
talloc_reference was replaced with a call to talloc_steal. This was in
preparation for moving to ralloc which doesn't support reference
counting.
The justification for talloc_steal within token_list_append in that
commit is that the tokens are being copied already. But the copies are
shallow, so this does not work.
Fortunately, the lifetime of these tokens is easy to understand. A
token list for "replacements" is created and stored in a hash table
when a function-like macro is defined. This list will live until the
macro is #undefed (if ever).
Meanwhile, a shallow copy of the list is created when the macro is
used and the list expanded. This copy is short-lived, so is unsuitable
as a new parent.
So we can just let the original, longer-lived owner continue to own
the underlying objects and things will work.
This fixes bug #45082:
"ralloc.c:78: get_header: Assertion `info->canary == 0x5A1106'
failed." when using a macro in GLSL
https://bugs.freedesktop.org/show_bug.cgi?id=45082
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
NOTE: This is a candidate for stable release branches.
This test cases exposes a bug as described in this bug report:
"ralloc.c:78: get_header: Assertion `info->canary == 0x5A1106'
failed." when using a macro in GLSL
https://bugs.freedesktop.org/show_bug.cgi?id=45082
Clearly, some memory is getting (incorrectly) freed on the first macro
invocation, leading to problems with the second macro invocation.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The trick here is that flex always chooses the rule that matches the most
text. So with a input text of "two:" which we want to be lexed as an
IDENTIFIER token "two" followed by an OTHER token ":" the previous OTHER
rule would match longer as a single token of "two:" which we don't want.
We prevent this by forcing the OTHER pattern to never match any
characters that appear in other constructs, (no letters, numbers, #,
_, whitespace, nor any punctuation that appear in CPP operators).
Fixes bug #44764:
GLSL preprocessor doesn't replace defines ending with ":"
https://bugs.freedesktop.org/show_bug.cgi?id=44764
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
NOTE: This is a candidate for stable release branches.
This demonstrates a bug that was recently triggered in piglit.
Here is the original bug report (containing a test case almost identical
to this one):
https://bugs.freedesktop.org/show_bug.cgi?id=44764
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Success was (tests-passed AND valgrind-tests-passed) but this meant that
if the valgrind tests weren't run it would be considered a failure.
The logic is now (tests-passed AND (!valgrind OR valgrind-tests-passed))
which lets us return success if the valgrind tests aren't run.
Reviewed-by: Eric Anholt <eric@anholt.net>
Tested-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Matt Turner <mattst88@gmail.com>
automake uses variables named *_SOURCES.
Reviewed-by: Eric Anholt <eric@anholt.net>
Tested-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Matt Turner <mattst88@gmail.com>
Exporting a publicly visible class with a generic name like
"variable_entry" via ir_variable_refcount.h is kind of mean.
Many IR transformers would like to define their own "variable_entry"
class. If they accidentally include this header, the compiler/linker
may get confused and try to instantiate the wrong variable_entry class,
leading to bizarre runtime crashes.
The hope is that renaming this one will allow .cpp files to safely
declare and use their own file-scope "variable_entry" classes.
This avoids crashes caused by converting src/glsl to automake.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-and-tested-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Fix this GCC warning on non-debug builds.
glsl_types.cpp: In member function 'gl_texture_index
glsl_type::sampler_index() const':
glsl_types.cpp:157: warning: control reaches end of non-void function
NOTE: This is a candidate for the 8.0 branch.
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Chad Versace <chad.versace@linux.intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The nvc0 gallium driver is advertising 128 MAX_INTERLEAVED_COMPS
which made it always assert in the linker when TFB was used since
the Outputs array was smaller than that maximum.
v2: added assertions
NOTE: This is a candidate for the 8.0 branch.
Reviewed-by: Paul Berry <stereotype441@gmail.com>
Commit ede60bc467 (glsl: Add isinf() and
isnan() builtins) uses "+INF" in the .ir file to represent infinity.
This worked on C99-compliant compilers, since the s-expression reader
uses strtod() to read numbers, and C99 requires strtod() to understand
"+INF". However, it didn't work on non-C99-compliant compilers such
as MSVC.
This patch modifies the s-expression reader to explicitly check for
"+INF" rather than relying on strtod() to support it.
This is a candidate for the 8.0 branch.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44767
Tested-by: Morgan Armand <morgan.devel@gmail.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
I copy-and-pasted the thing I was allocating for as the context, so
the first time it would be NULL (root of a ralloc context) and they'd
chain off each other from then on.
NOTE: This is a candidate for the 8.0 branch.
It used to be done in ir_to_mesa, and that was kind of a bad place.
I didn't change st_glsl_to_tgsi because there is some strange stuff
happening in the code that generates glDrawPixels shaders. It looked
like this would break horribly if I touched anything.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Track the calculated data in gl_shader_program instead of the
individual assembly shaders.
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Previous to this patch, we didn't do the limit check for
MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS until the end of the
store_tfeedback_info() function, *after* storing all of the transform
feedback info in the gl_transform_feedback_info::Outputs array. This
meant that the limit check wouldn't prevent us from overflowing the
array and corrupting memory.
This patch moves the limit check to the top of tfeedback_decl::store()
so that there is no risk of overflowing the array. It also adds
assertions to verify that the checks for
MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS and
MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS are sufficient to avoid
array overflow.
Note: strictly speaking this patch isn't necessary, since the maximum
possible number of varyings is MAX_VARYING (16), whereas the size of
the Outputs array is MAX_PROGRAM_OUTPUTS (64), so it's impossible to
have enough varyings to overflow the array. However it seems prudent
to do the limit check before the array access in case these limits
change in the future.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
On drivers that set gl_shader_compiler_options::LowerClipDistance (for
example i965), we need to handle transform feedback of gl_ClipDistance
specially, to account for the fact that the hardware represents it as
an array of vec4's rather than an array of floats.
The previous way this was accounted for (translating the request for
gl_ClipDistance[n] to a request for a component of
gl_ClipDistanceMESA[n/4]) doesn't work when performing transform
feedback on the whole unsubscripted array, because we need to keep
track of the size of the gl_ClipDistance array prior to the lowering
pass. So I replaced it with a boolean is_clip_distance_mesa, which
switches on the special logic that is needed to handle the lowered
version of gl_ClipDistance.
Fixes Piglit tests "EXT_transform_feedback/builtin-varyings
gl_ClipDistance[{1,2,3,5,6,7}]-no-subscript".
Reviewed-by: Eric Anholt <eric@anholt.net>
The function tfeedback_decl::num_components() was not correctly
accounting for transform feedback of whole arrays and gl_ClipDistance.
The bug was hard to notice in tests, because it only affected the
checks for MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS and
MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS.
This patch fixes the computation, and adds an assertion to verify
num_components() even when MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS
and MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS are not exceeded.
The assertion requires keeping track of components_so_far in
tfeedback_decl::store(); this will be useful in a future patch to fix
non-multiple-of-4-sized gl_ClipDistance.
Reviewed-by: Eric Anholt <eric@anholt.net>
We were doing the kill of the updated channels, then adding our copy
to the list of available stuff to copy. But if the copy was updating
its own source channels, we didn't notice, breaking this code:
R0.xyzw = arg0 + arg1;
R0.xyzw = R0.wwwx;
gl_FragColor.xyzw = clamp(R0.xyzw, 0.0, 1.0);
Fixes piglit glsl-copy-propagation-self-2.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
The trick was to produce an assignment in the IR along the lines of:
(assign (xyzw) (var_ref R0) (swiz wwww (var_ref R0) ))
which occurs only rarely even in code that looks like it should do
this, because of the assignment temporaries generated in ast_to_hir.
From the IR above, this optimization pass would then propagate
references of R0 into R0.wwww (seems reasonable), but without this
patch, a later reference of R0.wwww would see R0 first, turning that
into R0.wwww.wwww, which triggered opt_swizzle_swizzle, and then we
looped back to this code to do it again. Avoid that by skipping over
the usual ir_rvalue visitor's ir_swizzle hook, so that we get
handle_rvalue() on the ir_swizzle itself, not its referenced value.
Looking at only the swizzle will always optimize away at least as much
as looking at the swizzle's refererenced value.
We now still claim to propagate r0.w into r0.w, but at least we don't
trigger the loop.
v2: Rewrite commit message (changes by anholt)
Fixes piglit glsl-copy-propagation-self-1
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=34006
Previous to this patch, if the client requested transform feedback
using a subscript, but the variable was not an array
(e.g. "gl_FrontColor[0]"), we would produce a bogus error message like
"Transform feedback varying gl_FrontColor[0] found, but it's an array
([] expected)".
Changed the error message to e.g. "Transfrorm feedback varying
gl_FrontColor[0] requested, but gl_FrontColor is not an array."
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
The IR for mix(float, float, bool) was missing a write mask, causing the
IR reader to die horribly. Furthermore, I neglected to add any of the
new prototypes to the 1.30 profiles.
Fixes oglconform's glsl-bif-com advanced.mix test cases.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44477
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
The various l-value errors this was designed to catch are now caught
by other means. Marking the temporaries as read-only now just
prevents sensible error messages from being generated. It's
0:0(0): error: function parameter 'out p' references the read-only variable '_post_incdec_tmp'
versus
0:13(5): error: function parameter 'out p' references a post-decrement operation
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
Somethings, like pre-increment operations, were not previously caught.
After the 8.0 release, this code needs some major refactoring and
clean-up. It's a mess. :(
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Paul Berry <stereotype441@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755
This is similar to Gallium's existing glsl_to_tgsi::remove_output_read
lowering pass, but done entirely inside the GLSL compiler.
Signed-off-by: Vincent Lejeune <vljn@ovi.com>
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
It is not explicitly stated in the GL 3.0 spec that transform feedback
can be performed on a whole varying array (without supplying a
subscript). However, it seems clear from context that this was the
intent. Section 2.15 (TransformFeedback) says this:
When writing varying variables that are arrays, individual array
elements are written in order.
And section 2.20.3 (Shader Variables), says this, in the description
of GetTransformFeedbackVarying:
For the selected varying variable, its type is returned into
type. The size of the varying is returned into size. The value in
size is in units of the type returned in type.
If it were not possible to perform transform feedback on an
unsubscripted array, the returned size would always be 1.
This patch fixes the linker so that transform feedback on an
unsubscripted array is supported.
Fixes piglit tests "EXT_transform_feedback/builtin-varyings
gl_ClipDistance[{4,8}]-no-subscript" and
"EXT_transform_feedback/output_type *[2]-no-subscript".
Note: on back-ends that set
gl_shader_compiler_options::LowerClipDistance (for example i965),
tests "EXT_transform_feedback/builtin-varyings
gl_ClipDistance[{1,2,3,5,6,7}]" still fail. I hope to address this in
a later patch.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
On drivers that set gl_shader_compiler_options::LowerClipDistance (for
example i965), references to gl_ClipDistance (a float[8] array) will
be converted to references to gl_ClipDistanceMESA (a vec4[2] array).
This patch modifies the linker so that requests for transform feedback
of gl_ClipDistance are similarly converted.
Fixes Piglit test "EXT_transform_feedback/builtin-varyings
gl_ClipDistance".
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
When using transform feedback, there are three circumstances in which
it is useful for Mesa to instruct a driver to stream out just a
portion of a varying slot (rather than the whole vec4):
(a) When a varying is smaller than a vec4, Mesa needs to instruct the
driver to stream out just the first one, two, or three components of
the varying slot.
(b) In the future, when we implement varying packing, some varyings
will be offset within the vec4, so Mesa will have to instruct the
driver to stream out an arbitrary contiguous subset of the components
of the varying slot (e.g. .yzw or .yz).
(c) On drivers that set gl_shader_compiler_options::LowerClipDistance,
if the client requests that an element of gl_ClipDistance be streamed
out using transform feedback, Mesa will have to instruct the driver to
stream out a single component of one of the gl_ClipDistance varying
slots.
Previous to this patch, only (a) was possible, since
gl_transform_feedback_info specified only the number of components of
the varying slot to stream out. This patch adds
gl_transform_feedback_info::ComponentOffset, which indicates which
components should be streamed out.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Commit 9d36c96d6e (mesa: Fix
glGetTransformFeedbackVarying()) accidentally added an extra memset()
call to the store_tfeedback_info() function, causing
prog->LinkedTransformFeedback.NumBuffers to be erased.
This patch removes the extra memset and rearranges the other
operations in store_tfeedback_info() to be in the correct order.
Fixes piglit tests "EXT_transform_feedback/api-errors *unbound*"
Reviewed-by: Eric Anholt <eric@anholt.net>
The current implementation was totally broken -- it was looking in an
unpopulated structure for varyings, and trying to do so using the
current list of varying names, not the list used at link time.
v2: Fix leaking of memory into the program per re-link.
Reviewed-by: Paul Berry <stereotype441@gmail.com>
