If a query is made of a vector ssa_def (possibly from an intermediate
result), return all_bits. If a constant source is a vector, swizzle
the correct component.
Unit tests were added for the constant vector cases. I don't see a
great way to make unit tests for the other cases.
v2: Add a FINIHSME comment about u16vec2 hardware.
Fixes: 96303a59ea ("nir: Add some range analysis for used bits")
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9123>
There are patterns that will re-write the fmin or fmax part into a form
that other patterns will gradually convert to the same ior or iand. For
example,
fmax(b2f(a), b2f(b)) != 0
b2f(a || b) != 0
a || b
No shader-db or fossil-db changes on any Intel platform.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9122>
fmin(-A, -B) is -fmax(A, B), and fmax(-A, -B) is -fmin(A, B). Therefore
the logic joining A and B should toggle between ior and iand for the
negated versions.
At the very least, a shader from Euro Truck Simulator 2 in shader-db is
affected by this. The KIL instruction in the (ARB assembly) shader ends
up with the wrong logic. This is _probably_ the source of
https://gitlab.freedesktop.org/mesa/mesa/-/issues/1346.
That said, the issue mentions that Mesa 18.0.5 works, but commit
68420d8322 ("nir: Simplify min and max of b2f") was added in 17.3.
Moreover, I was not able to reproduce the error in the ETS2 shader from
shader-db from any Mesa commit near the time the original fd.o bugzilla
was submitted (December 2018). 🤷
In fact, the current error in that shader starts with 9167324a86
("nir/algebraic: Mark some logic-joined comparison reductions as
exact"). That's a bit of a red herring as 9167324a86 just sets off a
chain of replacements that eventually leads to the incorrect min/max of
b2f patterns fixed by this commit.
The other affected shaders in the shader-db results are from Cargo
Commander. These are also ARB assembly shaders.
I think any ARB assembly shader that uses the pattern
SLT r0, ...;
...
KIL -r0;
will suffer from issues related to this.
This change fixes the piglit
tests/spec/arb_fragment_program/kil-of-slt.shader_test test added in
https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/454.
shader-db results:
All Gen6+ platforms had similar result. (Ice Lake shown)
total instructions in shared programs: 20034604 -> 20034486 (<.01%)
instructions in affected programs: 3885 -> 3767 (-3.04%)
helped: 47
HURT: 2
helped stats (abs) min: 2 max: 4 x̄: 2.64 x̃: 2
helped stats (rel) min: 2.33% max: 8.33% x̄: 3.48% x̃: 3.39%
HURT stats (abs) min: 3 max: 3 x̄: 3.00 x̃: 3
HURT stats (rel) min: 13.64% max: 16.67% x̄: 15.15% x̃: 15.15%
95% mean confidence interval for instructions value: -2.83 -1.99
95% mean confidence interval for instructions %-change: -3.84% -1.60%
Instructions are helped.
total cycles in shared programs: 979881379 -> 979879406 (<.01%)
cycles in affected programs: 119873 -> 117900 (-1.65%)
helped: 46
HURT: 3
helped stats (abs) min: 10 max: 756 x̄: 45.41 x̃: 26
helped stats (rel) min: 0.53% max: 19.72% x̄: 1.67% x̃: 1.26%
HURT stats (abs) min: 28 max: 56 x̄: 38.67 x̃: 32
HURT stats (rel) min: 1.44% max: 3.54% x̄: 2.75% x̃: 3.27%
95% mean confidence interval for cycles value: -70.83 -9.70
95% mean confidence interval for cycles %-change: -2.23% -0.57%
Cycles are helped.
Iron Lake and GM45 had similar results. (Iron Lake shown)
total instructions in shared programs: 8115098 -> 8115076 (<.01%)
instructions in affected programs: 2592 -> 2570 (-0.85%)
helped: 32
HURT: 2
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.88% max: 2.70% x̄: 1.35% x̃: 1.31%
HURT stats (abs) min: 5 max: 5 x̄: 5.00 x̃: 5
HURT stats (rel) min: 17.24% max: 18.52% x̄: 17.88% x̃: 17.88%
95% mean confidence interval for instructions value: -1.15 -0.15
95% mean confidence interval for instructions %-change: -1.83% 1.39%
Inconclusive result (%-change mean confidence interval includes 0).
total cycles in shared programs: 238189718 -> 238189802 (<.01%)
cycles in affected programs: 75076 -> 75160 (0.11%)
helped: 3
HURT: 31
helped stats (abs) min: 2 max: 130 x̄: 44.67 x̃: 2
helped stats (rel) min: 0.18% max: 5.70% x̄: 2.02% x̃: 0.19%
HURT stats (abs) min: 2 max: 70 x̄: 7.03 x̃: 4
HURT stats (rel) min: 0.07% max: 6.41% x̄: 0.53% x̃: 0.15%
95% mean confidence interval for cycles value: -7.27 12.21
95% mean confidence interval for cycles %-change: -0.33% 0.94%
Inconclusive result (value mean confidence interval includes 0).
No fossil-db changes on any Intel platform.
Fixes: 68420d8322 ("nir: Simplify min and max of b2f")
Closes: #1346
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9122>
../src/compiler/glsl/gl_nir_link_uniforms.c:1786:50: warning: ‘blocks’ may be used uninitialized in this function [-Wmaybe-uninitialized]
1786 | const struct gl_uniform_block *const block =
Just do nothing if we somehow fail to initialize blocks.
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8724>
./src/compiler/glsl/link_uniforms.cpp: In member function ‘virtual void parcel_out_uniform_storage::visit_field(const glsl_type*, const char*, bool, const glsl_type*, glsl_interface_packing, bool)’:
../src/compiler/glsl/link_uniforms.cpp:1171:61: warning: ‘id’ may be used uninitialized in this function [-Wmaybe-uninitialized]
1171 | calculate_array_size_and_stride(prog, &this->uniforms[id],
../src/compiler/glsl/link_uniform_initializers.cpp: In function ‘gl_uniform_storage* linker::get_storage(gl_shader_program*, const char*)’:
../src/compiler/glsl/link_uniform_initializers.cpp:41:42: warning: ‘id’ may be used uninitialized in this function [-Wmaybe-uninitialized]
41 | return &prog->data->UniformStorage[id];
Can't happen, but flow control apparently can't prove it.
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8724>
../src/compiler/glsl/gl_nir_link_uniforms.c: In function ‘gl_nir_link_uniforms’:
../src/compiler/glsl/gl_nir_link_uniforms.c:1747:39: warning: ‘num_blocks’ may be used uninitialized in this function [-Wmaybe-uninitialized]
1747 | for (unsigned i = 0; i < num_blocks; i++) {
Don't have the patience to seee how you'd hit this but doing nothing is
clearly the right edge case behavior.
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8724>
This throws a curious warning:
In file included from ../src/compiler/nir/nir.h:32,
from ../src/compiler/nir/nir_opt_if.c:24:
../src/compiler/nir/nir_opt_if.c: In function ‘opt_if_loop_last_continue’:
../src/compiler/glsl/list.h:415:64: warning: ‘nif’ may be used uninitialized in this function [-Wmaybe-uninitialized]
415 | return !exec_list_is_empty(list) ? list->tail_sentinel.prev : NULL;
| ^
What's going on here is not enough of the optimizer has run to be able
to prove that nif is always initialized. So just handle the "can't
happen" case as if it could.
Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8724>
load_helper_invocation is an Input Builtin, for which the
value should not change during the execution of a shader.
This new pass inserts an is_helper intrinsic before any
demote() instruction and re-uses its value.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9058>
We already throw out any variables which may have a complex use so we
just need to make sure that our mode checks don't assert if we have a
deref which may_be but not must_be nir_var_function_temp.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9068>
We can't move the shuffle to a new block so this only works if the
shuffle and the bcsel are in the same block. Fortunately, in the
motivating case, this is true.
Also, we have to be careful around discard. We could try really hard to
just avoid moving them past discard but we choose to simply bail if we
see a discard instead.
Fixes: 4ff4d4e569 "nir/opt_intrinsic: Optimize bcsel(b, shuffle..."
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9068>
Previously, when we had a prototype-only function in SPIR-V, we would
compile it just fine and the function would have an impl that did
nothing. This commit changes that so that the nir_function::impl is
NULL to indicate a prototype-only function.
Reviewed-by: Jesse Natalie <jenatali@microsoft.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9069>
asan on llvmpipe with piglit tests/spec/arb_gl_spirv/execution/ssbo/array-indirect.shader_test
reported.
=================================================================
==3288325==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f5b2d6513cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf)
#1 0x7f5b2a1ae810 in ralloc_size ../src/util/ralloc.c:133
#2 0x7f5b2a1ae7e1 in ralloc_context ../src/util/ralloc.c:120
#3 0x7f5b2b210177 in gl_nir_link_uniform_blocks ../src/compiler/glsl/gl_nir_link_uniform_blocks.c:585
#4 0x7f5b2af7f52d in gl_nir_link_spirv ../src/compiler/glsl/gl_nir_linker.c:614
#5 0x7f5b2a3b76fa in st_link_nir ../src/mesa/state_tracker/st_glsl_to_nir.cpp:765
#6 0x7f5b2a3ace7b in st_link_shader ../src/mesa/state_tracker/st_glsl_to_ir.cpp:65
#7 0x7f5b2a471165 in _mesa_glsl_link_shader ../src/mesa/program/ir_to_mesa.cpp:3122
#8 0x7f5b2a97a6d8 in link_program ../src/mesa/main/shaderapi.c:1311
#9 0x7f5b2a97a6d8 in link_program_error ../src/mesa/main/shaderapi.c:1419
#10 0x7f5b2a97df45 in _mesa_LinkProgram ../src/mesa/main/shaderapi.c:1911
#11 0x7f5b299b59e5 in stub_glLinkProgram /mnt/devel/gl/piglit/tests/util/piglit-dispatch-gen.c:33956
#12 0x40a71a in link_and_use_shaders /mnt/devel/gl/piglit/tests/shaders/shader_runner.c:1604
#13 0x415722 in init_test /mnt/devel/gl/piglit/tests/shaders/shader_runner.c:5225
#14 0x4164ce in piglit_init /mnt/devel/gl/piglit/tests/shaders/shader_runner.c:5597
#15 0x7f5b29a214e9 in run_test /mnt/devel/gl/piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:73
#16 0x7f5b29a103fe in piglit_gl_test_run /mnt/devel/gl/piglit/tests/util/piglit-framework-gl.c:229
#17 0x407847 in main /mnt/devel/gl/piglit/tests/shaders/shader_runner.c:72
#18 0x7f5b2928f1e1 in __libc_start_main (/lib64/libc.so.6+0x281e1)
SUMMARY: AddressSanitizer: 48 byte(s) leaked in 1 allocation(s).
Fixes: 57239192 ("nir/linker: add gl_nir_link_uniform_blocks.c")
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8974>
The base mask previously used was 0xffffffff. This is not correct (but
should still work) for 16-bit and 8-bit values, but it means the high
32-bits of 64-bit values will get chopped off.
Instead of just restricting the pattern to 32-bits (as was done before
00b28a50b2), this extends the optimization in two ways:
1. Make it correct for other bit sizes.
2. Make it work for arbitrary shift counts.
This has the added benefit of reducing the number of patterns actually
added (7 previously, 4 now).
The "Reassociate for improved CSE" part is just reverted to its
pre-00b28a50b2c behavior. I doubt that pattern is likely to have much
impact outside 32-bits.
This change fixes the piglit tests
tests/spec/arb_gpu_shader_int64/fs-shl-of-shr-int64.shader_test and
tests/spec/arb_gpu_shader_int64/fs-iand-of-iadd-int64.shader_test.
All of the shaders helped in shader-db are vertex shaders on platforms
with vector-oriented vertex processing. The shaders contain ((x >> 16)
<< 16). These platforms set lower_extract_word, so the optimization
that transforms (x >> 16) to extract_u16 doesn't trigger. With only ~60
shaders involved, I didn't bother trying to add extract_XYZ versions of
these patterns to try to get those cases.
Fixes: 00b28a50b2 ("nir/algebraic: trivially enable existing 32-bit patterns for all bit sizes")
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Haswell and earlier Intel GPUs had simlar results. (Haswell shown)
total instructions in shared programs: 16397554 -> 16397496 (<.01%)
instructions in affected programs: 7961 -> 7903 (-0.73%)
helped: 58
HURT: 0
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 0.36% max: 1.89% x̄: 0.99% x̃: 0.78%
95% mean confidence interval for instructions value: -1.00 -1.00
95% mean confidence interval for instructions %-change: -1.13% -0.85%
Instructions are helped.
total cycles in shared programs: 1035483770 -> 1035483504 (<.01%)
cycles in affected programs: 75922 -> 75656 (-0.35%)
helped: 44
HURT: 2
helped stats (abs) min: 2 max: 12 x̄: 6.14 x̃: 2
helped stats (rel) min: 0.05% max: 1.67% x̄: 0.87% x̃: 0.72%
HURT stats (abs) min: 2 max: 2 x̄: 2.00 x̃: 2
HURT stats (rel) min: 0.06% max: 0.06% x̄: 0.06% x̃: 0.06%
95% mean confidence interval for cycles value: -7.28 -4.29
95% mean confidence interval for cycles %-change: -1.03% -0.63%
Cycles are helped.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8852>
OpEntryPoint declares the list of variables in Input and Output
storage classes that are used. Use that information to skip creating
other variables from such storage classes that are unused by the entry
point.
After that change, is not necessary to use remove dead variables for
those types of variables; and because of that is also not necessary to
lower initalizers for output variables.
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8456>
Basically every pass in NIR uses nir_ssa_def_rewrite_uses which calls
nir_instr_rewrite_src which is fairly complex because it handles all
sorts of non-SSA cases. Since we already know a priori that every
source written by nir_ssa_def_rewrite_uses is SSA, we can check new_src
once at the top of the function and cut out all that complexity.
While we're at it, we expose a new SSA-only nir_ssa_def_rewrite_uses_ssa
helper which takes an SSA def which avoids the one SSA check. It's also
more convenient 90% of the time.
Compile time as tested by Rhys Perry <pendingchaos02@gmail.com>
Difference at 95.0% confidence
-797.166 +/- 418.649
-0.566174% +/- 0.296441%
(Student's t, pooled s = 325.459)
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8790>
