Originally written by Ian Romanick for the Intel backend, but ported
to the new nir_opt_barrier_modes() common optimization pass. Ian's
original explanation and commit message follows:
Shared memory only exists within a workgroup, so synchronizing it beyond
workgroup scope is nonsense.
Basically every SPIR-V compiler generates operations like
OpMemoryBarrier(/*Memory*/Device,
/*Semantics*/AcquireRelease | WorkgroupMemory)
This is suggested in numerous places, including
https://github.com/KhronosGroup/GLSL/blob/master/extensions/khr/GL_KHR_vulkan_glsl.txt.
Even Mesa's glsl_to_nir pass does this. This advice, which has been
copy-and-pasted everywhere, is contrary to issue 13 in the original
GL_ARB_compute_shader spec:
"Since shared memory is only accessible to threads within a single
work group, memoryBarrierShared() also only requires synchronization
with other threads in the same work group."
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24842>
Many shaders issue full memory barriers, which may need to synchronize
access to images, SSBOs, shared local memory, or global memory.
However, many of them only use a subset of those memory types - say,
only SSBOs.
Shaders may also have patterns such as:
1. shared local memory access
2. barrier with full variable modes
3. more shared local memory access
4. image access
In this case, the barrier is needed to ensure synchronization between
the various shared memory operations. Image reads and writes do also
exist, but they are all on one side of the barrier, so it is a no-op for
image access. We can drop the image mode from the barrier here too.
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24842>
These patterns are broken in the following scenario:
%1 = f2fmp %0
%2 = fddx %1
%3 = ... // non quad uniform
if %3 {
%4 = f2f32 %2
...
}
Which would turn into
%3 = ...
if %3 {
%4 = fddx %0
...
}
Yet another example that shows why derivative instructions should be
be intrinsics, not alu.
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Emma Anholt <emma@anholt.net>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25014>
The condition
!param->type->is_vector() || !param->type->is_scalar()
alawys evaluates to true:
* type is not scalar or vector -> true
* type is vector, i.e. num_components > 1 -> num_components == 1 is
false and !is_scalar() == true
* type is scalar, i.e. num_components == 1 -> num_components > 1 is
false and !is_vector() == true
There is no comment explaining why such code has been written, therefore
this seems to be a mistake.
To maintain consistency with the surrounding code,
glsl_type_is_scalar_or_vector has been used instead of
replacing || with &&.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24914>
If undef is consumed by an FP opcode, replace it with NaN to eliminate
that opcode, else replace it with 0, but there are exceptions, such as
when undef is used by stores or phis, it's not touched.
This also contains workarounds for viewperf shaders.
radeonsi:
TOTALS FROM AFFECTED SHADERS (1987/58918)
Code Size: 5158692 -> 5143796 (-0.29 %) bytes
Max Waves: 22456 -> 22513 (0.25 %)
Outputs: 3726 -> 3726 (0.00 %)
Patch Outputs: 0 -> 0 (0.00 %)
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24059>
Dolphin ubershaders have a pattern:
if (x && y) {
} else {
discard;
}
The current code to simplify if's will bail on this pattern, since the condition
is not a comparison. However, if that check is dropped and we allow NIR to
invert this, we get:
if (!(x && y)) {
discard;
} else {
}
which is now in a form for nir_opt_conditional_discard to turn into it
discard_if(!(x && y))
which may be substantially cheaper than the original code.
In general, I see no reason to restrict to conditionals. Assuming the backend is
clever enough to delete empty else blocks (I think most are), then this patch is
a strict win as long as inot instructions are cheaper than empty else blocks.
This matches my intuition for typical GPUs, where simple ALU instructions are
cheaper than control flow. Furthermore, it may be possible in practice for
backends to fold the inot into a richer set of instructions. For example, most
GPUs have a NAND instructions which would fold in the inot in the above code.
So just drop the check, simplify the pass, get the win.
---
Also, to avoid inflating register pressure, make sure we put the inot right
before the if. Android shader-db on is uninspiring due to terrible
coalescing decisions in the current RA. But it does fix the Dolphin smell.
total instructions in shared programs: 1756571 -> 1756568 (<.01%)
instructions in affected programs: 1600 -> 1597 (-0.19%)
helped: 1
HURT: 4
Inconclusive result (value mean confidence interval includes 0).
total bytes in shared programs: 11521172 -> 11521156 (<.01%)
bytes in affected programs: 10080 -> 10064 (-0.16%)
helped: 1
HURT: 4
Inconclusive result (value mean confidence interval includes 0).
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24965>
In Vulkan, UBOs are lowered by nir_lower_explicit_io, and the ubo_base_agx
sysval is unused (since it doesn't handle descriptor sets). That makes the UBO
lowering GL-only and hence belongs with the GL driver rather than the compiler.
This lets us delete the ubo_base_agx sysval.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24847>
Doing a descriptor crawl with binding tables requires a real binding table in
the shader, which won't work for VK or merged shader stages in GL. Instead,
let's lower anything that needs a crawl to bindless in the driver, so the
compiler code doesn't need to know anything about descriptor binding models.
That gets rid of the texture_base_agx sysval, which is problematic when there
are multiple descriptor sets worth of textures.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24847>
For merging shader stages, it will be useful to express a load from an explicit
GL "descriptor set", so we can represent things like UBO loads with merged
shaders where UBOs can come from either stage. To do so, we add an intrinsic
representing a load from the driver's uniform tables, indexed like "descriptor
sets" with "bindings".
In principle, a layered GL-on-Vulkan implementation would use literal descriptor
sets for each stage, so I feel comfortable with the analogy here.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24847>
Instead of building a string, build a short struct type and use
that as key. The only caveat here is ensure there either there's
no internal padding or the internal padding is always the same.
Use a static assert to ensure we are in the former case.
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23281>
Kenneth Graunke