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>
