[Mesa-stable] [Mesa-dev] [PATCH 3/3] nv50/ir: fix combineLd/St to update existing records as necessary

Samuel Pitoiset samuel.pitoiset at gmail.com
Mon Jun 26 20:19:39 UTC 2017


These two are good finds!

Patch 2&3 are:

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

On 06/25/2017 12:39 AM, Ilia Mirkin wrote:
> Previously the logic would decide that the record is kept, which
> translates into keep = false in the caller, which meant that these
> passes did not run.
> 
> While it's right that keep = false which means that a new record does
> not need to be added, we do still have to perform the usual list
> maintenance. It's easiest to do this pre-merge rather than post.
> 
> The lowering that clip/cull distance passes produce triggers this bug in
> TCS (since reading outputs is done differently in other stages), but it
> should be possible to achieve it with the right sequence of regular
> reads/writes.
> 
> Fixes: KHR-GL45.cull_distance.functional
> Fixes: generated_tests/spec/arb_tessellation_shader/execution/tes-input/tes-input-gl_ClipDistance.shader_test
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>   src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 169436a4e39..57cb7ce214d 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -2485,6 +2485,10 @@ MemoryOpt::combineLd(Record *rec, Instruction *ld)
>   
>      assert(sizeRc + sizeLd <= 16 && offRc != offLd);
>   
> +   // lock any stores that overlap with the load being merged into the
> +   // existing record.
> +   lockStores(ld);
> +
>      for (j = 0; sizeRc; sizeRc -= rec->insn->getDef(j)->reg.size, ++j);
>   
>      if (offLd < offRc) {
> @@ -2541,6 +2545,10 @@ MemoryOpt::combineSt(Record *rec, Instruction *st)
>      if (prog->getType() == Program::TYPE_COMPUTE && rec->rel[0])
>         return false;
>   
> +   // remove any existing load/store records for the store being merged into
> +   // the existing record.
> +   purgeRecords(st, DATA_FILE_COUNT);
> +
>      st->takeExtraSources(0, extra); // save predicate and indirect address
>   
>      if (offRc < offSt) {
> 


More information about the mesa-stable mailing list