[Mesa-dev] [PATCH 06/12] nir/opt_peephole_select: Don't try to remove flow control around indirect loads

[Marek Olšák]

On our hardware, indirect accesses can go through memory with no
bounds checking.

Marek

On Thu, Jun 28, 2018 at 12:46 AM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> That flow control may be trying to avoid invalid loads.  On at least
> some platforms, those loads can also be expensive.
>
> No shader-db changes on any Intel platform (even with the later patch
> "intel/compiler: More peephole select").
>
> NOTE: I've tried to CC everyone whose drive might be affected by this
> change.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Marek Olšák <marek.olsak at amd.com>
> ---
>  src/compiler/nir/nir_opt_peephole_select.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_peephole_select.c b/src/compiler/nir/nir_opt_peephole_select.c
> index 4ca4f80d788..920ced2137c 100644
> --- a/src/compiler/nir/nir_opt_peephole_select.c
> +++ b/src/compiler/nir/nir_opt_peephole_select.c
> @@ -58,7 +58,8 @@
>   */
>
>  static bool
> -block_check_for_allowed_instrs(nir_block *block, unsigned *count, bool alu_ok)
> +block_check_for_allowed_instrs(nir_block *block, unsigned *count,
> +                               gl_shader_stage stage, bool alu_ok)
>  {
>     nir_foreach_instr(instr, block) {
>        switch (instr->type) {
> @@ -70,6 +71,13 @@ block_check_for_allowed_instrs(nir_block *block, unsigned *count, bool alu_ok)
>              switch (intrin->variables[0]->var->data.mode) {
>              case nir_var_shader_in:
>              case nir_var_uniform:
> +               /* Don't try to remove flow control around an indirect load
> +                * because that flow control may be trying to avoid invalid
> +                * loads.
> +                */
> +               if (nir_deref_has_indirect(stage, intrin->variables[0]))
> +                  return false;
> +
>                 break;
>
>              default:
> @@ -168,8 +176,10 @@ nir_opt_peephole_select_block(nir_block *block, nir_shader *shader,
>
>     /* ... and those blocks must only contain "allowed" instructions. */
>     unsigned count = 0;
> -   if (!block_check_for_allowed_instrs(then_block, &count, limit != 0) ||
> -       !block_check_for_allowed_instrs(else_block, &count, limit != 0))
> +   if (!block_check_for_allowed_instrs(then_block, &count, shader->info.stage,
> +                                       limit != 0) ||
> +       !block_check_for_allowed_instrs(else_block, &count, shader->info.stage,
> +                                       limit != 0))
>        return false;
>
>     if (count > limit)
> --
> 2.14.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev