CSE tries to collapse equal instructions, and collapsing two phis with incompatible dests is illegal.
Reviewed-by: Konstantin Seurer <konstantin.seurer@gmail.com>
Reviewed-by: Daniel Schürmann <daniel@schuermann.dev>
Reviewed-by: Jason Ekstrand <jason.ekstrand@collabora.com>
Fixes: 6bdce55c ("nir: Add a basic CSE pass")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19960>
The previous check assumed the stack starts at offset=0, which isn't
necessarily true for ray queries.
Note that this didn't cause correctness issues, just made an optimization
not apply. Found when I accidentally made this load-bearing in a
refactor.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20265>
Instead of calling like this:
st_draw_gallium -> cso_draw_vbo -> driver_draw_vbo
Do it like this:
st_draw_gallium -> driver_draw_vbo
OR
st_draw_gallium -> u_vbuf_draw_vbo
It's accomplished by adding a draw_vbo function pointer into cso_context.
The pointer is equal to pipe_context::draw_vbo when needed, so there is
no call overhead from this if cso's draw_vbo callback is indeed equal to
driver_draw_vbo. We just call cso_context_base::draw_vbo to jump into
the driver directly, or u_vbuf if needed.
The cso function with the indirect function call is inlined, so draws
don't actually visit any cso_context function.
Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20025>
While the hardware supports both counter and boolean occlusion queries, the
programming model is quite different from OpenGL. In AGX (and in Metal),
there is a single "visibility result buffer" associated with the render pass.
Each draw that uses occlusion queries writes into this render pass global
visibility result buffer at a particular index. By contrast, the OpenGL
occlusion query model supposes that each query has independent state that can be
mixed and matched within a render pass. We can't simply allocate backing memory
for a query and write to it from a job. We can't allocate visibility result
buffers for each batch up front and statically assign OpenGL queries to indices,
because the OpenGL query can span multiple batches. Finally we can't use a
global visibility result buffer without introducing additional synchronization,
given that we now support multiple batches in-flight at once.
In this patch, I've elected to use a simple solution: allocate visibility result
buffers and indices on the fly as needed, and accumulate the results on the CPU
at the end of the render pass. When we have proper synchronization we'll want to
revisit this, but as everything is stalling at submit time now, I'm not inclined
to "optimize" something I can't test.
Passes dEQP-GLES3.functional.occlusion_query.* and the relevant piglit tests.
The piglits are considerably more thorough, checking lots of "we hate tilers"
conditions that dEQP skips over.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20123>
First remove agx_shader_key from asahi_shader_key. It's trivial. agx_shader_key
is going to go away soon now that we lower everything in NIR.
Then everything else is mutually exclusive between stages. That means much
less to hash. drawoverhead test 1 from 2331 to 3443.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20123>
From Panfrost. This lets us avoid storing depth/stencil attachments at the end
of the frame in GLES.
On my 4K monitor, glmark2 -btexture at fullscreen goes from 705fps to 1150fps. I
assume gains on real workloads will be smaller.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20123>
Adapted from panfrost, with the work happening at CSO create time instead of
draw time allowing us to do more sophisticated analysis. We'll use these for
accurate masks in a moment.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20123>
Unusual hardware features that require special hanlding usually get a
devinfo field, so do this for MTL's unordered DF types. This will
guarantee that any platform based on MTL (thus inheriting from
MTL_FEATURES) will automatically be handled in these special cases.
v2: s/has_unordered_64bit_float/has_64bit_float_via_math_pipe/ (Curro).
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
In the previous patch we adjusted the scoreboard pass to take into
consideration a new case of unordered operations for TGL. Fix the
decoding as well.
v2: use intel_device_info_is_mtl() (Curro, Jordan)
v3: the part where we export num_sources_from_inst() is now a separate patch
(Curro).
v4: Work around false positive maybe-unitialized warning since Marge
uses -Werror=maybe-uninitialized (Marge).
Reviewed-by: Francisco Jerez <currojerez@riseup.net> (v3)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
We want to call this from brw_disasm.c, so move it out to brw_eu.c
since it's about to become more of a shared utility function than
something specific to the EU validator.
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
On MTL, instructions with DF type are unordered, executed in the math
pipe. This means that they require different SWSB dependency handling,
and also that in some cases such as MOVs it's generally faster to
simply use 2 smaller ordered moves than a single unordered MOV.
One problem we have with the current code is that generate_code() is
not setting the proper SWSB dependencies for the generated DF MOVs,
causing some tests to fail.
One solution would be to fix generate_code() by making it set the
appropriate dependencies. This was the first patch I wrote. Another
solution to this problem, pointed to us by Curro, is to change
required_exec_type() so we use UD instructions instead of DF, just
like we do with platforms that don't have 64 bit instructions, which
means there won't be anything to fix in generate_code(). The second
solution is what this patch implements.
This fixes at least:
- dEQP-VK.subgroups.arithmetic.framebuffer.subgroupmin_double_vertex
Thanks to Francisco Jerez for all the major help provided with this
problem.
Credits-to: Francisco Jerez <currojerez@riseup.net>
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
When we use this form there's no way to specify which pipe RegDist
refers to, so there are a few rules to figure this out, which is what
inferred_sync_pipe() implements. But for MTL there's no long pipe and
the documentation does not explicitly explain what should be the
inferred type for its long (DF) instructions - which are out-of-order,
by the way. One way to interpret this is that such case should be
avoided. So add the extra check to entirely avoid this case.
Notice that this is not actually fixing any bug, since returning
TGL_PIPE_LONG (what we do today) will actually make these DF
instructions incompatible with every in-order instruction, so we'll
never opt to use the (RegDist, SBID) form anyway. But still, it's
better to have this case explicitly documented instead of having it
covered by a semi coincidence.
v2: use intel_device_info_is_mtl() (Curro, Jordan)
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
Adjust the scoreboard code to take that into account.
Fixes at least:
- dEQP-VK.glsl.builtin.precision_double.refract.compute.vec3
- dEQP-VK.glsl.builtin.precision_double.matrixcompmult.compute.mat4
v2: use intel_device_info_is_mtl() (Curro, Jordan)
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20072>
Fix all_mrt_write_mask when independent_blend_enable is false.
Otherwise, lrz write is always diabled with MRT when
independent_blend_enable is false. This fixes a 2% perf regression for
multiple gfxbench benchmarks.
Fixes: 0132c22de7 ("freedreno/a6xx: Don't disable LRZ for invalid channels")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20254>
We were promoting reads with a valid primary to direct CPU maps even
if the mmap mode was IRIS_MMAP_WC, which would mean uncached reads from
VRAM. In that case, GPU blits are in fact useful!
We were also only checking for !DISCARD_RANGE rather than MAP_READ,
which isn't a great idea for image maps, given the discussion in the
previous commit about image map semantics.
The original code was also just confusingly structured. Make a helper
function with clearly defined cases where we want to bail on CPU maps.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19209>
When performing transfer maps on images that require staging buffers
(say, for presenting a linear view of tiled memory), we were reading
the existing contents of the buffer into the staging resource on map
unless PIPE_MAP_DISCARD_RANGE was set.
The thinking was to support partial writes. If you map a subrectangle
of an image, but then only write selective pixels - should it preserve
the existing contents of the mapped region? I believed that it should,
unless you pass PIPE_MAP_DISCARD_RANGE to explicitly say that that it's
okay to invalidate the destination region.
However, that does not appear to be the interpretation favored by other
Mesa developers (in particular Michel Dänzer and Marek Olšák). The
radeonsi driver does not do this readback from the destination region
to the staging buffer unless you pass PIPE_MAP_READ. If you want to
do a partial write and preserve contents, you need to pass both flags:
(PIPE_MAP_READ | PIPE_MAP_WRITE). Passing READ is expected to come
with an associated cost.
OpenGL defines GL_MAP_INVALIDATE_RANGE_BIT for mapping buffer objects,
which is translated to PIPE_MAP_DISCARD_RANGE. However, unextended
OpenGL doesn't define mapping textures. There are two main sources of
image maps: our internal MapTextureImage() hook, and gbm_bo_map().
I've audited our internal MapTextureImage() calls, and while some do
pass PIPE_MAP_DISCARD_RANGE, almost all of them wholly overwrite the
mapped region, and those that care about combining with existing image
contents all pass PIPE_MAP_READ. So this should work there.
GBM defines three flags: GBM_BO_TRANSFER_READ, WRITE, and READ_WRITE.
There is no defined "invalidate range" bit. In issue #6020, Matthias
Treydte notes that this extra readback can cause performance problems,
and with iris's current interpretation, there's no way to avoid it.
During that discussion, Michel and Matthias both argued that
GBM_BO_TRANSFER_WRITE should invalidate the destination contents and
avoid the readback, while GBM_BO_TRANSFER_READ_WRITE would preserve it.
This patch makes iris follow that model for image mappings, removing
readback on staging maps for both detiling and stall avoidance, unless
PIPE_MAP_READ is passed. I believe we can change this with impunity.
For buffer objects, Ian Romanick and I both agree that partial writes
should be supported, and GL_MAP_INVALIDATE_RANGE_BIT exists precisely
to indicate that you should spend effort preserving existing contents.
So we continue doing readback for buffers unless PIPE_MAP_DISCARD_RANGE
is flagged, for now. While I think this is work, it also seems to be
undertested in the CTS and Piglit.
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6020
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19209>
If we successfully replace the backing storage for a buffer, we know
that it's idle, and the transfer map code can mark it unsynchronized
right away, letting us skip redundant resource_is_busy() checks.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19209>
We already gave out the old BO...or acquired it from somewhere which
may be affecting it. We simply can't replace the backing store.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19209>
