The key change is to use a builder to write the expected shader result
and compare that. To make this less error prone, a few helper functions
were added
- a way to allocate VGRFs from both shaders in parallel, that way the
same brw_reg can be used in both of them;
- assertions that a pass will make progress or not, and proper output
when the unexpected happens;
- use a common brw_shader_pass_test class so to collect some of the helpers;
- make some helpers work directly with builder.
The idea is to improve the signal in tests, so that the disasm comments
are not necessary anymore. For example
```
TEST_F(saturate_propagation_test, basic)
{
brw_reg dst1 = bld.vgrf(BRW_TYPE_F);
brw_reg src0 = bld.vgrf(BRW_TYPE_F);
brw_reg src1 = bld.vgrf(BRW_TYPE_F);
brw_reg dst0 = bld.ADD(src0, src1);
set_saturate(true, bld.MOV(dst1, dst0));
/* = Before =
*
* 0: add(16) dst0 src0 src1
* 1: mov.sat(16) dst1 dst0
*
* = After =
* 0: add.sat(16) dst0 src0 src1
* 1: mov(16) dst1 dst0
*/
brw_calculate_cfg(*v);
bblock_t *block0 = v->cfg->blocks[0];
EXPECT_EQ(0, block0->start_ip);
EXPECT_EQ(1, block0->end_ip);
EXPECT_TRUE(saturate_propagation(v));
EXPECT_EQ(0, block0->start_ip);
EXPECT_EQ(1, block0->end_ip);
EXPECT_EQ(BRW_OPCODE_ADD, instruction(block0, 0)->opcode);
EXPECT_TRUE(instruction(block0, 0)->saturate);
EXPECT_EQ(BRW_OPCODE_MOV, instruction(block0, 1)->opcode);
EXPECT_FALSE(instruction(block0, 1)->saturate);
}
```
becomes
```
TEST_F(saturate_propagation_test, basic)
{
brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16);
brw_builder exp = make_shader(MESA_SHADER_FRAGMENT, 16);
brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg dst1 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg src0 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg src1 = vgrf(bld, exp, BRW_TYPE_F);
bld.ADD(dst0, src0, src1);
bld.MOV(dst1, dst0)->saturate = true;
EXPECT_PROGRESS(brw_opt_saturate_propagation, bld);
exp.ADD(dst0, src0, src1)->saturate = true;
exp.MOV(dst1, dst0);
EXPECT_SHADERS_MATCH(bld, exp);
}
```
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33936>
While nullity of the CheckedPtr object is checked, writing to a raw
pointer safely requires that several other invariants be satisfied, so
it should be marked as unsafe to reflect that.
v2: reorder commits for cherry-picking and remove alignment check
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33989>
The cl_slice version of this function checks several invariants for us,
providing a slight safety net.
v2: reordered commits to allow cherry-picking bugfixes
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33989>
std::slice::from_raw_parts requires that the slice pointer be non-null,
even when the slice contains zero elements. Failing this invariant is
undefined behavior.
v2: reordered commits to allow cherry-picking bugfixes
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33989>
clGetSupportedImageFormats will write as many supported formats as are
discovered at present, regardless of the value of num_image_formats.
This could result in writing out-of-bounds memory.
v2: reordered commits to allow cherry-picking bugfixes
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Cc: stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33989>
This test already passed when executed standalone, but hit a issue triggered
by switching between fast and slow ZS clears when executed together with other
dEQP tests. This issue has been fixed by the previous commit, so we can drop
the fail from the CI expectation now.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34029>
When a slow/fast/slow clear sequence is executed on a surface, the second
slow clear will not regenerate the clear command if the clear value of the
fast clear is the same as the one used for the second slow clear, as the
current stored surface clear value is the same as the new clear value.
The command generated on the first slow clear however may have used a
different clear value, which is now submitted unchanged to the hardware on
the second slow clear.
Fix this by only generating the clear command if there is no valid one
already. If we already have a valid clear command simply update the fill
value in that command with the new clear value. This has some marginal
overhead, but has been chosen over the alternative of adding more state by
remembering the last slow clear value.
Cc: mesa-stable
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34029>
The problem occurs with a series of instructions build the subgroup
invocation value :
mov(8) g23<1>UW 0x76543210V
add(8) g23.8<1>UW g23<8,8,1>UW 0x0008UW
add(16) g23.16<1>UW g23<16,16,1>UW 0x0010UW
Our register spilling code operates on physical registers (64B on
Xe2+) and using the brw_inst::is_partial_write() helper only considers
32B registers. So the spiller doesn't see that the add(16) instruction
is doing a partial write and ends up discarding the previous value.
You can reproduce the issue by running a test like :
INTEL_DEBUG=spill_fs ./deqp-vk -n dEQP-VK.compute.pipeline.cooperative_matrix.khr_a.subgroupscope.constant.uint8_uint8.buffer.rowmajor.linear
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: aa494cbacf ("brw: align spilling offsets to physical register sizes")
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33642>
With all the RT-enabled driver setting this field, we can now have the
runtime use it instead of calling into the driver's vfunc.
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34026>
Cayman has a non-deterministic behavior issue which is
visible with the test below (arb_shader_image_size).
The tests fail randomly at the "fragment" test category.
Anyway, if the "compute" category is removed, the same
tests are working flawlessly.
The "compute" part of the driver was interfering with the
graphic pipeline. The culprit is the packet PKT3_DEALLOC_STATE
which puts the gpu in an incorrect state to perform some
graphic operations.
This change fixes this problem by issuing a PKT3_CLEAR_STATE
packet just after the PKT3_SURFACE_SYNC packet. As explained
by d51dbe048a PKT3_DEALLOC_STATE is mandatory on cayman to
avoid a gpu hang at the PKT3_SURFACE_SYNC stage.
This correction makes tests like
"spec@glsl-4.30@execution@built-in-functions@cs-.*" to pass
in an utterly deterministic way without random failures.
This change removes around 500 random failures for a
"piglit run all".
For instance, this issue is triggered on cayman with
"piglit/bin/arb_shader_image_size-builtin -auto -fbo".
Fixes: d51dbe048a ("r600g/compute: Emit DEALLOC_STATE on cayman after dispatching a compute shader.")
Signed-off-by: Patrick Lerda <patrick9876@free.fr>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33973>
We use common impl here. Supporting debug utils would require lots of
interceptions in venus, so we'd not go that far but would only expose
debug report. Better than nothing.
Signed-off-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34037>
Venus can only enable VK_EXT_display_control after using common vk_sync,
unless we add new layered implementation in common. Like how I replaced
the common android present impl, but no bandwidth at this point.
Fixes: 89ec6c4d8f ("venus: add a few more trivial extensions")
Signed-off-by: Yiwei Zhang <zzyiwei@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34037>
This optimization seems broken because eg. v_s_log_f32 uses SGPRs
for both the source and destination but applying OMOD seems to require
VGPRs.
This fixes a GPU hang when launching Enshrouded on GFX1201.
No fossils db changes on GFX1201.
Cc: mesa-stable
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34027>
The premise of the change was wrong: the case where the defs analysis
was required was rare and requiring the analysis inside just the
case we care was being used for another analysis too. So for now,
the change doesn't really helps. I'll revisit this whole pass later on.
This backs out commit 6e19215810.
Acked-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34039>
A lot of people (including me) misinterpreted the conformanceVersion
field for so long. The Vulkan spec wasn't very clear either but it's
going to be clarified soon.
VkConformanceVersion is actually unrelated to the official CTS
conformance process in Khronos. It just reports the latest CTS version
that the driver can pass, not more.
For GFX8+, RADV should be passing CTS 1.4.0.0 on all GPUs because we
validated this CTS version recently for Vulkan 1.4.
For GFX6-7, which only suppports Vulkan 1.3, RADV should also be
passing CTS 1.4.0.0, because newer versions of the CTS can be used
to validate a driver against an older version of the spec, so
it's perfectly fine to report a higher CTS version than the Vulkan version.
Newer CTS versions likely can't pass 100% due to a DGC bug that I still
need to fix.
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12799
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34018>
Instead of calling `require()` every instruction, call it once per pass.
Even though the defs are cached (i.e. we are not re-calculating them every
instruction), this prevents the extra check and the call to analysis
validation.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34010>
Otherwise, the size of the EGLSurface and the drawable may get out of
sync if kopper needs to re-create the swapchain at a different size.
This can cause problems with things like eglSetDamageRegionKHR() where
the core EGL code clamps them to the size in the EGLSurface.
With Wayland, it's up to the client to choose a size and resize by
creating a new EGLSurface with a different size. Only on X11 can we
get a resize side-band like this.
Normally, without kopper, this goes the other direction where the X11
EGL code will detect a surface size change in dri2_x11_query_surface()
and it invalidates the drawable if they've changed, forcing
re-allocation. Kopper, however, works more like the DRI2 path where we
just get handed buffers at some size decided by X11 and have to deal
with them. In the DRI2 path, the size is unconditionally updated by
dri2_x11_get_buffers(). This is roughly equivalent, updating the size
right after every call to kopperSwapBuffers().
Fixes: 8ade5588e3 ("zink: add kopper api")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12797
Reviewed-By: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34015>
This should fix some hangs on polaris when decode is forced on.
Fixes: 95a980b61f ("radv/video: add event support for VCN4")
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34013>
- brw_lower_3src_null_dest: Allocating a new destination, so include
INSTRUCTION_DATA_FLOW class.
- brw_lower_alu_restriction: Removing instruction, so include
INSTRUCTION_IDENTITY. No details are changed so remove
INSTRUCTION_DETAIL.
- brw_lower_vgrfs_to_fixed_grfs: Changing source and destination
numbers, so include INSTRUCTION_DETAIL.
- brw_lower_send_gather: Insert new instructions (scalar register) and
change sources and other information on existing ones. So include
INSTRUCTION_DETAIL and INSTRUCTION_IDENTITY. Promote to INSTRUCTIONS.
- brw_opt_eliminate_find_live_channel: Can change source, so include
INSTRUCTION_DATA_FLOW.
- brw_opt_copy_propagation_defs and brw_opt_cse_defs: Both can remove
instructions, so include INSTRUCTION_IDENTITY. Promote to
INSTRUCTIONS.
- brw_opt_saturate_propagation: Instruction can have `sat` modified,
and operands can have type modified, so include INSTRUCTION_DETAIL.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33993>
