If we did, we would have the instruction coming right after ldvary write
to the same implicit destination as ldvary at the same time. We prevent
this when merging instructions, but we should make sure we prevent this
when we move ldvary around for pipelining too.
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13921>
According to the spec the hardware locks the scoreboard on the first
or last thread switch (selected via shader state) and any TLB accesses
executed before this are not synchronized by hardware.
This change updates the logic to ensure we respect this requirement
and that we don't assume that the lock is acquired automatically
on the first TLB access, which is not valid at least since V3D 4.1+.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13910>
Writes to physical registers are not allowed after thread end. We
were checking this for ALU writes, but we need to check it for
signal writes too.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13910>
This solves a case where a NIR geometry shader was storing the output in
a non-constant:
vec4 32 ssa_1 = load_const (0xc0800000 /* -4.000000 */, 0xc1100000 /* -9.000000 */, 0x40400000 /* 3.000000 */, 0x40e00000 /* 7.000000 */)
vec1 32 ssa_7 = load_const (0x00000000 /* 0.000000 */)
vec1 32 ssa_8 = load_const (0x00000001 /* 0.000000 */)
vec1 32 ssa_9 = iadd ssa_7, ssa_8
vec1 32 ssa_19 = mov ssa_1.x
intrinsic store_output (ssa_19, ssa_9) (1, 1, 0, 160, 288) /* base=1 */ /* wrmask=x */ /* component=0 */ /* src_type=float32 */ /* location=32 slots=2 gs_streams(x=0 y=0 z=0 w=0) */
When lowering the VPM output we check if the destination (ssa_9 in this
case) is a constant to add to the VPM offset. We run a constant folding
optimization in an earlier VS lowering, and we should do the same for
GS.
This fixes multiple dEQP-VK.pipeline.interface_matching.* failures.
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13884>
While fragment and geometry shader were handling structs as inputs, they
weren't doing for it arrays of structures.
This fixes multiple dEQP-VK.pipeline.interface_matching.* failures and
assertions.
v2:
- Fix style (Iago).
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13884>
When early fragment tests are mandated by the shader, we must use
the Z value produced by the FEP even if there are elements that
would typically require late fragment tests (such as discards,
sample to coverage, etc).
This change means we also need to be a bit more careful when
we promote shaders to use early fragment tests so we don't
promote anything with discards for example.
Fixes:
dEQP-VK.fragment_operations.early_fragment.discard_early_fragment_tests_depth
dEQP-VK.fragment_operations.early_fragment.discard_early_fragment_tests_stencil
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13837>
We had been storing pointers to a driver owned swizzle table
rather than storing the actual swizzle value in various shader
and pipeline keys on both GL and Vulkan drivers.
This doesn't look very robust, particularly since we also
compute sha1 hashes from these values and we may store these
hashes to disk (for the disk cache).
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13738>
Typically, optimization passes go through all the blocks in a shader
and make adjustments on the fly, so we always want them to update
the current block or the current block pointer will become outdated.
Also, we don't need to keep track of the previous current block
pointer to restore it, since optimization passes run after we have
completed conversion to VIR, and therefore, anything that comes after
that should always set the current block before emitting code.
Fixes debug assert crashes when running shader-db:
vir.c:1888: try_opt_ldunif: Assertion `found || &c->cur_block->instructions == c->cursor.link' failed
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13625>
This was not quite correct in that our checks for the allowed cases
were not checking that there were no other peripheral access other
than the ones allowed.
For example, we allowed wrtmuc signal and TMU write other than
TMUC, and we also allowed TMU read and VPM read/write. But we
cannot allow wrtmuc with TMU write other than TMUC and at the
same time a VPM write for example, so we can't just check if we
have a combination of allowed peripherals, we still need to check
that those are the only ones in use by the combined instructions.
Another example is that even if we allow a TMU write (other than TMUC)
with a wrtmuc signal, the resulting instruction must still have just
one TMU write other than TMUC, but we were allowing the merge if one
instruction signaled wrtmuc and the other wrote to tmu other than tmuc
without testing if the combined result would have 2 tmu writes.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13527>
This was not considering the possibility that the driver has called
nir_before_block() or nir_after_block() to update the cursor, in which
case the cursor link points to the instruction list header and not
to an actual instruction.
Fixes incorrect debug-assert crash in:
dEQP-VK.graphicsfuzz.cov-increment-vector-component-with-matrix-copy
Fixes: 265515fa62 ("broadcom/compiler: check instruction belongs to current block")
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13467>
A TSY barrier becomes effective at the point of the next thread switch,
so if we have one coming after a previous thread switch we need to
be careful not to emit it in its delay slots, or we would be effectively
moving the barrier earlier than intended.
Fixes simulator assert crash in:
dEQP-VK.graphicsfuzz.two-for-loops-with-barrier-function
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13468>
It is really small, and used just twice, so we just call qpu_magic.
We also update how it is used:
* QFILE_NULL is an undef so we can just load anything. Previously we
were using accumulator 0, but there isn't any real reason to use
an accumulator for this. Using reg 0.
* QFILE_LOAD_IMM: it seems that we don't use at all right now, so
let's add an assert
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13008>
As we don't know if we are going to have spilling or not, emit always a
last thrsw at the end of the shader.
If later we don't have spillings and we don't need that last thrsw, we
remove it and switch back to the previous one.
This way we ensure all the spilling happens always before the last
thrsw.
v2 (Juan):
- Rework the code to force a last thrsw and remove later if no spilling
v3:
- Merge functionality inside vir_emit_last_thrsw (Iago)
- Add vir_restore_last_thrsw (Juan)
v4 (Iago):
- Fix/add new comments
- Rename variables/parameters
v5 (Iago):
- Fix comments
- Add assertion
Cc: mesa-stable
Fixes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4760
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12322>
This function is only used in v3d_nir_to_vir(), so make it private.
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12322>
We had an optimization to auto-enable early fragment tests when a shader
didn't have side effects, but of course, we cannot do that this if the
shader writes Z, as in that case the fragment tests need to use the
value written from the shader.
Also, if the shader enables early fragment tests, then any shader Z
writes should be ignored.
Fixes:
dEQP-VK.spirv_assembly.instruction.graphics.early_fragment.*
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12736>
Fix defect reported by Coverity Scan.
Same on both sides (CONSTANT_EXPRESSION_RESULT)
pointless_expression: The expression inst->qpu.flags.auf !=
V3D_QPU_UF_NONE || inst->qpu.flags.auf != V3D_QPU_UF_NONE does not
accomplish anything because it evaluates to either of its identical
operands, inst->qpu.flags.auf != V3D_QPU_UF_NONE.
Fixes: 3f2c54a27f ("broadcom/compiler: rewrite partial update liveness tracking")
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12385>
When incrementing unifa address in DCE optimization, ensure that we
setup correctly the current block, so the ldfunif optimization is also
executed correctly.
This fixes
dEQP-VK.graphicsfuzz.cov-struct-float-array-mix-uniform-vectors
heap-buffer overflow with address sanitizer enabled.
v2 (Iago):
- Save and restore current block
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12339>
The code we had for this was a work in progress and not finished. Also,
it was geared towards partial writes caused by output packing (i.e.
fp16) and was ignoring partial updates caused by conditional writes,
which are far more common in our case.
This change provides an implementation for tracking conditional writes
that works in tandem with the previous spill change to narrow liveness
for their spills.
Fixes register allocation failures in:
dEQP-VK.graphicsfuzz.spv-stable-maze-flatten-copy-composite
We also gain one shader from shader-db:
total instructions in shared programs: 13339969 -> 13338584 (-0.01%)
instructions in affected programs: 185520 -> 184135 (-0.75%)
helped: 375
HURT: 130
Instructions are helped.
total threads in shared programs: 412038 -> 412040 (<.01%)
threads in affected programs: 2 -> 4 (100.00%)
helped: 1
HURT: 0
total uniforms in shared programs: 3746581 -> 3746585 (<.01%)
uniforms in affected programs: 49 -> 53 (8.16%)
helped: 0
HURT: 1
total max-temps in shared programs: 2359960 -> 2359947 (<.01%)
max-temps in affected programs: 289 -> 276 (-4.50%)
helped: 7
HURT: 0
Max-temps are helped.
total sfu-stalls in shared programs: 34351 -> 34359 (0.02%)
sfu-stalls in affected programs: 218 -> 226 (3.67%)
helped: 35
HURT: 37
Inconclusive result (value mean confidence interval includes 0).
total inst-and-stalls in shared programs: 13374320 -> 13372943 (-0.01%)
inst-and-stalls in affected programs: 186653 -> 185276 (-0.74%)
helped: 373
HURT: 132
Inst-and-stalls are helped.
LOST: 0
GAINED: 1
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12278>
A spill of a conditional write generates code like this:
mov.ifa t5000, 0
mov tmud, t5000
nop t5001; ldunif (0x00008100 / 0.000000)
add tmua, t11, t5001
Here, we are spilling t5000, which has a conditional write, and we
produce an inconditional spill for it. This implicitly means that
our spill requires a correct value for all channels of t5000.
If we do a conditional spill, then we emit:
mov.ifa t5000, 0
mov tmud.ifa, t5000
nop t5001; ldunif (0x00008100 / 0.000000)
add tmua.ifa, t11, t5001
Which only uses channels of t5000 that have been written by the
instruction being spilled.
By doing the latter, we can then narrow down the liveness for t5000
more effectively, as we can use this to detect that the block only reads
(in the tmud instruction) the values that have been written previously
in the same block (in the mov instruction). This means that values in
other channels are not used, and therefore, we don't need them to be
alive at the start of the block. This means that if this is the only
write of t5000 in this block, we can consider that the block
completely defines t5000.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12278>
The spill base setting instructions (which includes some uniforms) are
added in the entry block, not in the current block. When ldunif
optimization is applied, the cursor is pointing to instructions in the
entry block, but the current block is a different one. This leads to a
heap-buffer-overflow when going through the list of instructions
(detected by the address sanitizer).
Thus change the current block to entry block, and restore it after the
setup is done.
This fixes
dEQP-VK.ssbo.readonly.layout.single_struct.single_buffer.std430_instance_array_comp_access_store_cols
with address sanitizer enabled.
v2:
- Set current block instead of disabling ldunif optimization (Iago)
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12221>
This is where it should be rather than having to pass it into the
optimisation pass every time.
It also allows us to call the loop analysis pass without having to
duplicate these options which we will do later in this series.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12064>
Like in the case of emitting a block, process pending TMU operations
before a jump is executed.
Fixes dEQP-VK.graphicsfuzz.stable-binarysearch-tree-nested-if-and-conditional.
Fixes: 197090a3fc ("broadcom/compiler: implement pipelining for general
TMU operations")
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11971>
OpenGL ES 3.1 specifies that a geometry shader can write to gl_PrimitiveID,
which can then be read by a fragment shader.
OpenGL ES 3.2 additionally adds the capacity for the fragment shader
to read gl_PrimitiveID even if there is no geometry shader. This
commit adds support for this feature, which is also implicitly
expected by the geometry shader feature in Vulkan 1.0.
Fixes:
dEQP-VK.pipeline.framebuffer_attachment.no_attachments
dEQP-VK.pipeline.framebuffer_attachment.no_attachments_ms
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11874>
Clip distance arrays will come as compact array variables, so we need
to handle them as such, like we did for vertex inputs.
Fixes:
dEQP-VK.clipping.user_defined.clip_distance.vert_geom.{1,2,3,4}
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11783>
For some reason, this was implemented with the bulk of the compute
shader enablement, but this intrinsic is specific to subgroups and
thus was not really used. Also, its implementation was not correct,
since it was returning the element index within the subgroup, not
the subgroup index itself, which is the index of the batch in the
dispatch.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11620>
The intrinsic produces a vec2, so let's honor that and avoid the weird
lowering to scalar and later reconstruction to vec2 when we find
load vulkan descriptor intrinsics.
It fixes tests like this (which require that we expose KHR_spirv_1_4):
dEQP-VK.spirv_assembly.instruction.spirv1p4.opptrequal.null_comparisons_ssbo_equal
that otherwise produce bad code that tries to access a vec2 from the result of
that intrinsic, leading to NIR validation errors.
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11257>
