| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
| |
i915 is mixing the use of these fields, for now change this to a
struct and add a FIXME.
Reviewed-by: Kenneth Graunke <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99229
|
|
|
|
|
| |
Reviewed-by: Eduardo Lima Mitev <[email protected]>
Reviewed-by: Jordan Justen <[email protected]>
|
|
|
|
| |
This matches the naming of nir_lower_vars_to_ssa, the other to-SSA pass.
|
|
|
|
|
|
|
|
|
| |
After removing brw_shader in the previous commit this is no longer
needed.
V2: remove use in src/compiler/glsl/test_optpass.cpp
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
| |
This allows us to delete brw_shader and removes the last use of
gl_linked_shader in the codegen paths.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will let us to make _CurrentFragmentProgram a gl_program pointer
allowing for simpilifications to be made.
We also need to add a field to gl_shader to hold it during parsing.
In gl_program we put it inside a union in anticipation of moving
more fields here that can be only fs or vertex stage fields.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
| |
gl_shader_program
This will allow us to make the CurrentProgram array store gl_program which allows
us to do a bunch of simplifications.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will help allow us to store gl_program in the CurrentProgram array rather
than gl_shader_program which will allow a bunch of simplifications.
Note that we make LinkedTransformFeedback a pointer so we don't waste
memory creating a struct for each stage. We also store a pointer to
the gl_program that will contain the pointer in gl_shader_program so
we can get easy access to the correct stage.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
| |
We have already set the gl_shader_program pointer to the correct
shader program in _mesa_BeginTransformFeedback() so use it.
This is more consistent with how we do it for gen7.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
| |
We no longer need to initialise it because gl_program is never reused.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
| |
This will be used in api_validate.c in a following patch when we
switch to using gl_program pointers for the pipelines CurrentProgram
array.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
| |
This now contains everything we need.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
| |
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
| |
This will allow us to store gl_program rather than gl_shader_program
as the current program perstage which allows us to simplify code
that makes use of the CurrentProgram list.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will allow us to simplify the current program logic for SSO.
Also since we aim to detach shader_info from nir_shader this will come
in handy avoiding passing nir_shader around just to keep track of
the stage we are dealing with.
V2: set stage for arb asm programs also.
Reviewed-by: Eric Anholt <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use atomic ops when updating gl_shader::RefCount.
Fixes intermittent failures and crashes in
'dEQP-EGL.functional.sharing.gles2.multithread.*'.
All tests in that group now pass except
'dEQP-EGL.functional.sharing.gles2.multithread.simple_egl_server_sync.textures.copyteximage2d_texsubimage2d_render'.
Tested with:
mesa: branch 'master' at d6545f2
deqp: branch 'nougat-cts-dev' at 4acf725 with additional local fixes
DEQP_TARGET: x11_egl
hw: Intel Broadwell 0x1616
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99085
Reviewed-by: Timothy Arceri <[email protected]>
Reviewed-by: Tapani Pälli <[email protected]>
Cc: [email protected]
Cc: Mark Janes <[email protected]>
Cc: Haixia Shi <[email protected]>
|
|
|
|
|
|
|
| |
To my knowledge, this fixes no tests. I simply wrote the patch for
completeness as a follow-up to the previous two patches.
Reviewed-by: Tapani Pälli <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
_mesa_choose_tex_format() already handles GL_RGBA + GL_UNSIGNED_SHORT_1_5_5_5_REV
by converting it to MESA_FORMAT_B5G5R5A1_UNORM. Teach it do the same for
the non-reversed type. Otherwise, the switch's fallthrough converts it
to an 8888 format, which has incompatible precision in the alpha
channel.
Patch 2/2 to fix dEQP-EGL.functional.image.modify.tex_rgb5_a1_tex_subimage_rgba8
on Intel.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99185
Cc: Haixia Shi <[email protected]>
Reviewed-by: Tapani Pälli <[email protected]>
Cc: "13.0" <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This allows eglCreateImage() to accept textures of said format.
Patch 1/2 to fix
dEQP-EGL.functional.image.modify.tex_rgb5_a1_tex_subimage_rgba8
on Intel.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99185
Cc: Haixia Shi <[email protected]>
Reviewed-by: Tapani Pälli <[email protected]>
Cc: "13.0" <[email protected]>
|
|
|
|
|
|
|
|
|
| |
In this case we were dying when we tried to do SHL addr sampler imm(8)
because that puts an immediate in src0 of a two source instruction. This
fixes 2704 of the new separate sampler Vulkan CTS tests on Sky Lake.
Reviewed-by: Eduardo Lima Mitev <[email protected]>
Cc: "13.0" <[email protected]>
|
|
|
|
| |
Reviewed-by: Timothy Arceri <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes two new piglit tests:
spec/glsl-1.10/execution/vs-nested-return-sibling-loop.shader_test
spec/glsl-1.10/execution/vs-nested-return-sibling-loop2.shader_test
shader-db results for BDW:
total instructions in shared programs: 12903158 -> 12903134 (-0.00%)
instructions in affected programs: 27100 -> 27076 (-0.09%)
helped: 32
HURT: 6
total cycles in shared programs: 294922518 -> 294922804 (0.00%)
cycles in affected programs: 4372828 -> 4373114 (0.01%)
helped: 31
HURT: 8
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There is a single regression in loop unrolling which is:
loops HURT: shaders/orbital_explorer.shader_test GS SIMD8: 0 -> 1
However the loop is huge so it seems reasonable not to unroll it. It's
surprising that GLSL IR does unroll it.
shader-db results BDW:
total instructions in shared programs: 13037455 -> 13036947 (-0.00%)
instructions in affected programs: 17982 -> 17474 (-2.83%)
helped: 63
HURT: 25
total cycles in shared programs: 262217870 -> 262227990 (0.00%)
cycles in affected programs: 2287046 -> 2297166 (0.44%)
helped: 969
HURT: 844
total loops in shared programs: 2951 -> 2952 (0.03%)
loops in affected programs: 0 -> 1
helped: 0
HURT: 1
LOST: 0
GAINED: 1
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
shader-db results for BDW:
total instructions in shared programs: 12589614 -> 12590119 (0.00%)
instructions in affected programs: 50525 -> 51030 (1.00%)
helped: 7
HURT: 145
total cycles in shared programs: 241524604 -> 241490502 (-0.01%)
cycles in affected programs: 1941404 -> 1907302 (-1.76%)
helped: 302
HURT: 449
total loops in shared programs: 4245 -> 2947 (-30.58%)
loops in affected programs: 1535 -> 237 (-84.56%)
helped: 1142
HURT: 0
total spills in shared programs: 14453 -> 14453 (0.00%)
spills in affected programs: 0 -> 0
helped: 0
HURT: 0
total fills in shared programs: 18984 -> 18984 (0.00%)
fills in affected programs: 0 -> 0
helped: 0
HURT: 0
LOST: 26
GAINED: 15
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
| |
Later we will pass compiler to nir_optimise to be used by the loop unroll
pass.
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This moves the nir_lower_indirect_derefs() call into
brw_preprocess_nir() so thats is called by both OpenGL and Vulkan
and removes that call to the old GLSL IR pass
lower_variable_index_to_cond_assign()
We want to do this pass in nir to be able to move loop unrolling
to nir.
There is a increase of 1-3 instructions in a small number of shaders,
and 2 Kerbal Space program shaders that increase by 32 instructions.
The changes seem to be caused be the difference in the GLSL IR vs
NIR variable index lowering passes. The GLSL IR pass creates a
simple if ladder for arrays of size 4 or less, while the NIR pass
implements a binary search for all arrays regardless of size.
Shader-db results BDW:
total instructions in shared programs: 13021176 -> 13021819 (0.00%)
instructions in affected programs: 57693 -> 58336 (1.11%)
helped: 20
HURT: 190
total cycles in shared programs: 299805580 -> 299750826 (-0.02%)
cycles in affected programs: 2290024 -> 2235270 (-2.39%)
helped: 337
HURT: 442
total fills in shared programs: 19984 -> 19984 (0.00%)
fills in affected programs: 0 -> 0
helped: 0
HURT: 0
LOST: 4
GAINED: 0
V2: remove the do_copy_propagation() call from the i965 GLSL IR
linking code. This call was added in f7741c52111 but since we are
moving the variable index lowering to NIR we no longer need it and
can just rely on the nir copy propagation pass.
Reviewed-by: Kenneth Graunke <[email protected]>
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Without this we will regress the max-samplers piglit test on Gen6
and lower when loop unrolling is done in NIR. There is a check
in the GLSL IR linker that errors when it finds indirects and
EmitNoIndirectSampler is set.
As far as I can tell there is no reason for not enabling this for
all gens regardless of whether they fully support ARB_gpu_shader5
or not.
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
| |
The GL 4.5 spec says:
"If any enabled array’s buffer binding is zero when DrawArrays
or one of the other drawing commands defined in section 10.4 is
called, the result is undefined."
This commits avoids crashing the code, which is not a very good
"undefined result".
This fixes spec/!opengl 3.1/vao-broken-attrib piglit test.
|
|
|
|
|
|
|
|
| |
Use an offloading thread for all nine_context functions.
Macros are used to ease the reading of the code.
Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Axel Davy <[email protected]>
|
|
|
|
|
|
|
| |
DRI_CONF_NINE_OVERRIDEVENDOR was missing gettext for the
description.
Signed-off-by: Axel Davy <[email protected]>
|
|
|
|
|
|
| |
See the patch for the new controls added.
Signed-off-by: Axel Davy <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
BaseVertex, BaseInstance, DrawID, and some edge flag conditions need
vertex buffer and elements structs. We can't bail early in this case.
Gen4-7 already do this properly. Gen8+ did not.
Thanks to Ilia Mirkin for helping track this down.
Cc: [email protected]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99144
Reported-by: Pierre-Eric Pelloux-Prayer <[email protected]>
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Commit 929fcee47e46781c57f2a354ce0a013915c033d1 introduced code that
attempts to unlock an unlocked mutex which is undefined behaviour.
On OpenBSD this leads to an abort:
0 0x0000124dadfa96ba in thrkill () at <stdin>:2
1 0x0000124dadf3da39 in *_libc_abort () at /usr/src/lib/libc/stdlib/abort.c:52
2 0x0000124d2c1165b5 in *_libpthread_pthread_mutex_unlock (mutexp=<optimized out>)
at /usr/src/lib/librthread/rthread_sync.c:221
3 0x0000124d279c02e4 in init_attrib_groups (ctx=0x124df0fda000) at main/context.c:825
4 _mesa_initialize_context (ctx=ctx@entry=0x124df0fda000, api=api@entry=API_OPENGL_CORE,
visual=visual@entry=0x7f7ffffbdfd0, share_list=share_list@entry=0x0,
driverFunctions=driverFunctions@entry=0x7f7ffffbda60) at main/context.c:1204
5 0x0000124d27b507ec in st_create_context (api=api@entry=API_OPENGL_CORE,
pipe=pipe@entry=0x124dc4910000, visual=visual@entry=0x7f7ffffbdfd0,
share=share@entry=0x0, options=options@entry=0x7f7ffffbe128)
at state_tracker/st_context.c:545
6 0x0000124d27b8639f in st_api_create_context (stapi=<optimized out>,
smapi=0x124d1b608800, attribs=0x7f7ffffbe100, error=0x7f7ffffbe0fc, shared_stctxi=0x0)
at state_tracker/st_manager.c:669
7 0x0000124d27cc5b9c in dri_create_context (api=<optimized out>, visual=0x124d8a0f8a00,
cPriv=0x124de473f240, major_version=<optimized out>, minor_version=<optimized out>,
flags=<optimized out>, notify_reset=false, error=0x7f7ffffbe2b4,
sharedContextPrivate=0x0) at dri_context.c:123
8 0x0000124d27cc5029 in driCreateContextAttribs (screen=0x124d8a0f8400,
api=<optimized out>, config=0x124d8a0f8a00, shared=<optimized out>,
num_attribs=<optimized out>, attribs=<optimized out>, error=0x7f7ffffbe2b4,
data=0x124d77814a00) at dri_util.c:448
9 0x0000124d8e109b00 in drisw_create_context_attribs (base=0x124df3e08700,
config_base=0x124d7a0e7300, shareList=<optimized out>, num_attribs=<optimized out>,
attribs=<optimized out>, error=0x7f7ffffbe2b4) at drisw_glx.c:476
10 0x0000124d8e104b4a in glXCreateContextAttribsARB (dpy=0x124d533f0000,
config=0x124d7a0e7300, share_context=0x0, direct=1, attrib_list=0x7f7ffffbe300)
at create_context.c:78
Signed-off-by: Jonathan Gray <[email protected]>
Reviewed-by: Eduardo Lima Mitev <[email protected]>
|
|
|
|
|
|
|
|
|
|
| |
It's possible that nir_shader was cloned and it no longer contains
a pointer to the shader_info in gl_program. So we need to copy
shader_info back to gl_program if that is the case.
Fixes a regression with NIR_TEST_CLONE=true
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98840
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
main/dlist.c: In function ‘save_DrawArraysInstancedARB’:
main/dlist.c:1748:36: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawArraysInstancedARB(GLenum mode,
^~~~
main/dlist.c:1749:35: warning: unused parameter ‘first’ [-Wunused-parameter]
GLint first,
^~~~~
main/dlist.c:1750:37: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1751:37: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount)
^~~~~~~~~
main/dlist.c: In function ‘save_DrawElementsInstancedARB’:
main/dlist.c:1759:38: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawElementsInstancedARB(GLenum mode,
^~~~
main/dlist.c:1760:39: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1761:38: warning: unused parameter ‘type’ [-Wunused-parameter]
GLenum type,
^~~~
main/dlist.c:1762:45: warning: unused parameter ‘indices’ [-Wunused-parameter]
const GLvoid *indices,
^~~~~~~
main/dlist.c:1763:39: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount)
^~~~~~~~~
main/dlist.c: In function ‘save_DrawElementsInstancedBaseVertexARB’:
main/dlist.c:1771:48: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawElementsInstancedBaseVertexARB(GLenum mode,
^~~~
main/dlist.c:1772:49: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1773:48: warning: unused parameter ‘type’ [-Wunused-parameter]
GLenum type,
^~~~
main/dlist.c:1774:55: warning: unused parameter ‘indices’ [-Wunused-parameter]
const GLvoid *indices,
^~~~~~~
main/dlist.c:1775:49: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount,
^~~~~~~~~
main/dlist.c:1776:47: warning: unused parameter ‘basevertex’ [-Wunused-parameter]
GLint basevertex)
^~~~~~~~~~
main/dlist.c: In function ‘save_DrawArraysInstancedBaseInstance’:
main/dlist.c:1785:45: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawArraysInstancedBaseInstance(GLenum mode,
^~~~
main/dlist.c:1786:44: warning: unused parameter ‘first’ [-Wunused-parameter]
GLint first,
^~~~~
main/dlist.c:1787:46: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1788:46: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount,
^~~~~~~~~
main/dlist.c:1789:45: warning: unused parameter ‘baseinstance’ [-Wunused-parameter]
GLuint baseinstance)
^~~~~~~~~~~~
main/dlist.c: In function ‘save_DrawElementsInstancedBaseInstance’:
main/dlist.c:1797:47: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawElementsInstancedBaseInstance(GLenum mode,
^~~~
main/dlist.c:1798:48: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1799:47: warning: unused parameter ‘type’ [-Wunused-parameter]
GLenum type,
^~~~
main/dlist.c:1800:52: warning: unused parameter ‘indices’ [-Wunused-parameter]
const void *indices,
^~~~~~~
main/dlist.c:1801:48: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount,
^~~~~~~~~
main/dlist.c:1802:47: warning: unused parameter ‘baseinstance’ [-Wunused-parameter]
GLuint baseinstance)
^~~~~~~~~~~~
main/dlist.c: In function ‘save_DrawElementsInstancedBaseVertexBaseInstance’:
main/dlist.c:1810:57: warning: unused parameter ‘mode’ [-Wunused-parameter]
save_DrawElementsInstancedBaseVertexBaseInstance(GLenum mode,
^~~~
main/dlist.c:1811:58: warning: unused parameter ‘count’ [-Wunused-parameter]
GLsizei count,
^~~~~
main/dlist.c:1812:57: warning: unused parameter ‘type’ [-Wunused-parameter]
GLenum type,
^~~~
main/dlist.c:1813:62: warning: unused parameter ‘indices’ [-Wunused-parameter]
const void *indices,
^~~~~~~
main/dlist.c:1814:58: warning: unused parameter ‘primcount’ [-Wunused-parameter]
GLsizei primcount,
^~~~~~~~~
main/dlist.c:1815:56: warning: unused parameter ‘basevertex’ [-Wunused-parameter]
GLint basevertex,
^~~~~~~~~~
main/dlist.c:1816:57: warning: unused parameter ‘baseinstance’ [-Wunused-parameter]
GLuint baseinstance)
^~~~~~~~~~~~
Signed-off-by: Ian Romanick <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
| |
Signed-off-by: Ian Romanick <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
| |
v2: Set linearizer_array_index in process_block_array_leaf. Suggested
by Timothy.
Signed-off-by: Ian Romanick <[email protected]>
Cc: [email protected]
Reviewed-by: Timothy Arceri <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
arb_program_interface_query/arb_program_interface_query-resource-query,
and GL45-CTS.program_interface_query.separate-programs-{tess-control,
tess-eval,geometry}. Only one dEQP program interface failure remains.
I would have liked to split this up into several distinct changes, but
I wasn't sure how to do that given thet tangled nature of these issues.
So, the issues:
* We need to treat interface blocks declared as an array of instances
as a single block - removing the outer array. The resource list
entry's name should not include the array length. Properties such
as GL_ARRAY_SIZE should refer to the variable inside the block, not
the interface block's array properties.
* We need to do this prefixing even for structure variables.
* We need to do this for built-ins (such as gl_PerVertex.gl_Position).
* After interface array unwrapping, any variable which is an array
should have [0] appended. It doesn't matter if it's a TCS/TES/GS
input or TCS output - that looked like an attempt to unwrap for
per-vertex variables, but that didn't consider per-patch variables,
and as far as I can tell there's nothing to justify this.
Several Mesa developers have suggested that Issue 16 contradicts the
main specification, but I believe that it doesn't - the main spec just
isn't terribly clear. The main ARB_program_interface query spec says:
"* For an active interface block not declared as an array of block
instances, a single entry will be generated, using the block name from
the shader source.
* For an active interface block declared as an array of instances,
separate entries will be generated for each active instance. The name
of the instance is formed by concatenating the block name, the "["
character, an integer identifying the instance number, and the "]"
character."
Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
but several people suggested the second bullet above means that it
should be named "gl_PerVertex[array length].gl_Position".
There are two important things to note. Those bullet points say
"an active interface block", while the others say "variable" or "active
shader storage block member". They also don't mention applying the
rules recursively (unlike the other bullets). Both suggest that
these rules apply to blocks themselves, not members of blocks.
In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
"block[1]", ... resource list entries - so those rules are real,
and actually used. So if they don't apply to block members, then how
should members be named? Unfortunately, I don't see any rules outside
of issue 16 - where the rationale is very unclear. I hope to clarify
the spec in the future.
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Alejandro Piñeiro <[email protected]>
Reviewed-by: Ian Romanick <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These are listed as Z+ in the GL spec, and often have values of
0xFFFFFFFF. For glGetFloat, we should return 4294967295.0 rather than
-1.0. Similarly, for glGetInteger64v, we should return 0xFFFFFFFF, not
the sign extended 0xFFFFFFFFFFFFFFFF.
Fixes 6 dEQP tests matching the pattern
dEQP-GLES3.functional.state_query.integers.stencil*value*mask*getfloat
when run in a single process (with state reset code happening between
tests, which makes dEQP set the stencil value mask to 0xFFFFFFFF).
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Ilia Mirkin <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The "State Tables" section of the OpenGL specification lists many values
as belonging to Z+ (non-negative integers), not Z (all integers).
For ordinary glGetInteger queries, this doesn't matter. However, when
accessing Z+ values via glGetFloat or glGetInteger64, we need to treat
the source value as an unsigned value. Otherwise, we'll produce a
negative number when bit 31 is set.
This commit merely adds the plumbing. It doesn't convert any values.
v2: Gotta catch 'em all (add missing cases caught by Ilia)
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Ilia Mirkin <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
GetFloat of integer valued things is supposed to perform a simple
int -> float conversion. INT_TO_FLOAT is not that. Instead, it
converts [-2147483648, 2147483647] to a normalized [-1.0, 1.0] float.
This is only used for COMPRESSED_TEXTURE_FORMATS, which nobody in
their right mind would try and access via glGetFloat(), but we may
as well fix it.
Found by inspection.
v2: Gotta catch 'em all (fix another case of this caught by Ilia)
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Ilia Mirkin <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We want to perform the URB read to a vec4 temporary, with no writemask,
then issue a MOV to swizzle the data and store it to the actual
destination, using the final writemask.
We were doing this wrong. For example, let's say we wanted to read
a vec2 stored in components 2-3 of a vec4. We would generate a URB
read message of:
SEND <actual destination>.XY <header with mask set to XY>
MOV <actual destination>.XY <actual destination>.ZW
This doesn't work, because the URB message reads the .XY components
of the vec4, rather than the ZW. It writes to the right place, but
with the wrong data. Then the MOV comes along and overwrites it
with data that didn't even come from the URB at all.
Instead we want to do:
SEND <temporary> <header with mask set to ZW>
MOV <actual destination>.XY <temporary>.ZW
Signed-off-by: Kenneth Graunke <[email protected]>
Reviewed-by: Jason Ekstrand <[email protected]>
|
|
|
|
| |
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
| |
Not used anymore. It was just a scalar MOV.
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
| |
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Asking the DC for less than one cacheline (4 owords) of data for
uniform pull constants is suboptimal because the DC cannot request
less than that from L3, resulting in wasted bandwidth and unnecessary
message dispatch overhead, and exacerbating the IVB L3 serialization
bug. The following table summarizes the overall framerate improvement
(with statistical significance of 5% and sample size ~10) from the
whole series up to this patch for several benchmarks and hardware
generations:
| SKL | BDW | HSW
SynMark2 OglShMapPcf | 24.63% ±0.45% | 4.01% ±0.70% | 10.31% ±0.38%
GfxBench4 gl_manhattan31 | 5.93% ±0.35% | 3.92% ±0.31% | 6.62% ±0.22%
GfxBench4 gl_4 | 2.52% ±0.44% | 1.23% ±0.10% | N/A
Unigine Valley | 0.83% ±0.17% | 0.23% ±0.05% | 0.74% ±0.45%
Note that there are two versions of the Manhattan demo shipped with
GfxBench4, one of them is the original gl_manhattan demo which doesn't
use UBOs, so this patch will have no effect on it, and another one is
the gl_manhattan31 demo based on GL 4.3/GLES 3.1, which this patch
benefits as shown above.
I haven't observed any statistically significant regressions in the
benchmarks I have at hand. Note that the comparatively huge
improvement on SKL in the OglShMapPcf test case is due to the combined
effect of this patch and the register pressure benefit on SKL+ of
"i965/fs: Switch to the constant cache for uniform pull constants.",
part of the same series.
Going up to 8 oword blocks would improve performance of pull constants
even more, but at the cost of some additional bandwidth and register
pressure, so it would have to be done on-demand based on the number of
constants actually used by the shader.
v2: Fix for Gen4 and 5.
v3: Non-trivial rebase. Rework to allow the visitor specifiy
arbitrary pull constant block sizes.
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
| |
Change the FS generator to ask the dataport for enough owords worth of
constants to fill the execution size of the instruction -- Which means
that the visitor now needs to set the execution size correctly for
uniform pull constant load instructions, which we were kind of
neglecting until now.
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
| |
We'll need roughly the same logic in other places and it would be
annoying to duplicate it. Instead factor it out into a function-like
macro that takes the number of dwords per block (which will prove more
convenient than taking the same value in owords or some other unit).
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts to using the oword block read messages for uniform pull
constant loads, as used to be the case until
4c1fdae0a01b3f92ec03b61aac1d3df5. There are two important differences
though: Now the L3 cacheability bits are set up correctly for UBOs
(since 11f5d8a5d4fbb861ec161f68593e429cbd65d1cd), and we target the
constant cache instead of the data cache. The latter used to get no
L3 way allocation on boot on all platforms that existed at the time,
so oword read messages wouldn't get cached on L3 regardless of the
MOCS bits, what probably explains the apparent slowness of oword
fetches.
Constant cache loads seem to perform better than SIMD4x2 sampler loads
in a number of cases, they alleviate some of the cache thrashing
caused by the competition with textures for the L1/L2 sampler caches,
and they allow fetching up to 128B worth of constants with a single
oword fetch message.
Note that IVB devices suffer from a hardware bug that leads to
serialization of L3 read requests overlapping the same cacheline as
result of a (on IVB buggy) mechanism of the L3 to preserve coherency.
Since read requests for matching cachelines from any L3 client are not
pipelined, throughput may decrease in cases where there are no
non-overlapping requests left in the queue that can be processed
between them.
This situation should be relatively uncommon as long as we make sure
that we don't use the 1/2 oword messages in cases where the shader
intends to read from any other location of the same cacheline at some
other point. This is generally a good idea anyway on all generations
because using the 1 and 2 oword messages is expected to waste
bandwidth since the minimum L3 request size for the DC is exactly 4
owords (i.e. one cacheline). A future commit will have this effect.
I haven't been able to find any real-world example where this would
still result in a regression on IVB, but if someone happens to find
one it shouldn't be too difficult to add an IVB-specific check to have
it fall back to the sampler cache for pull constant loads.
Note that on SKL+ this change has the additional benefit of reducing
the register footprint of pull constant loads. The following table
summarizes the effect of the whole series on several shader-db stats:
Total instructions Total cycles
BWR: 4571248 -> 4568342 (-0.06%) 123375740 -> 123373296 (-0.00%)
ELK: 3989020 -> 3985402 (-0.09%) 98757068 -> 98754058 (-0.00%)
ILK: 6383591 -> 6376787 (-0.11%) 143649910 -> 143648914 (-0.00%)
SNB: 7528395 -> 7501446 (-0.36%) 103503796 -> 102460370 (-1.01%)
IVB: 6949221 -> 6943317 (-0.08%) 60592262 -> 60584422 (-0.01%)
HSW: 6409753 -> 6403702 (-0.09%) 60609070 -> 60604414 (-0.01%)
BDW: 8043467 -> 7976364 (-0.83%) 68427730 -> 68483042 (0.08%)
CHV: 8045019 -> 7977916 (-0.83%) 68297426 -> 68352756 (0.08%)
SKL: 8204037 -> 7939086 (-3.23%) 66583900 -> 65624378 (-1.44%)
Lost->Gained Total spills Total fills
BWR: 5 -> 5 1488 -> 1488 (0.00%) 1957 -> 1957 (0.00%)
ELK: 5 -> 5 1489 -> 1489 (0.00%) 1958 -> 1958 (0.00%)
ILK: 1 -> 4 1449 -> 1449 (0.00%) 1921 -> 1921 (0.00%)
SNB: 0 -> 0 549 -> 549 (0.00%) 52 -> 52 (0.00%)
IVB: 13 -> 3 1271 -> 1271 (0.00%) 1162 -> 1162 (0.00%)
HSW: 11 -> 0 1271 -> 1271 (0.00%) 1162 -> 1162 (0.00%)
BDW: 12 -> 0 1340 -> 1340 (0.00%) 1452 -> 1452 (0.00%)
CHV: 12 -> 0 1340 -> 1340 (0.00%) 1452 -> 1452 (0.00%)
SKL: 0 -> 120 1269 -> 375 (-70.45%) 1563 -> 690 (-55.85%)
v3: Non-trivial rebase.
Reviewed-by: Kenneth Graunke <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
brw_set_dp_read_message already had a target_cache argument, but its
interpretation was rather convoluted (on Gen6 the render cache was
used if the caller asked for it, otherwise it was ignored using the
sampler cache instead), and the constant cache wasn't representable at
all. brw_set_dp_write_message used the data cache on Gen7+ except for
RENDER_TARGET_WRITE messages, in which case it would use the render
cache. On Gen6 the render cache was always used.
Instead of the above, provide the shared unit SFID that the caller
expects will be used. Makes no functional changes.
v3: Non-trivial rebase.
Reviewed-by: Kenneth Graunke <[email protected]>
|