Re: [Mesa-dev] [PATCH 00/19] AMD support for NIR deref instructions.

[Rob Clark]

On Mon, May 14, 2018 at 8:07 PM, Dave Airlie  wrote:
> On 13 May 2018 at 10:19, Bas Nieuwenhuizen  wrote:
>> This implements support in radv and radeonsi for NIR deref
>> instructions instead of deref chains.
>>
>> It contains 4 parts:
>> - patch 1 is a fixup for the initial series by Jason
>> - Add support everywhere for instruction based derefs
>> - Stop lowering them to deref chains (may need to be synchronized
>>   with other drivers and core nir code to keep things bisectable)
>> - Remove support for deref chains.
>
> Okay I've read over this and I'm pretty happy with it.
>
> I do wonder if we could lower the impact on some of the code by
> adding the wrappers functions and getting var in a few places.
>
> i.e. instead of
> +  nir_variable *var = uses_deref_chain ? instr->variables[0]->var :
> +
> nir_deref_instr_get_variable(nir_instr_as_deref(instr->src[0].ssa->parent_instr);
> -   int idx = instr->variables[0]->var->data.driver_location;
> +   int idx = var->data.driver_location;

I guess the uses_deref_chain part goes away, but I'd proposed some
helper to get the var before.. it saves a bit of the churn, but not
all.. (and iirc we ended up with a helper something along those lines
anyways, I'd have to switch back to deref branch to check the name..

Anyways, I'm pretty happy with Jason's series.. I didn't really bother
to try to review it patch by patch, but it is a lot of churn so
looking at the end result seemed to be the more sensible way to
approach it.  I've played with it quite a bit since I've added pointer
support for compute on top.

I guess tomorrow I should rebase the gallium/ir3/vc4/vc5 parts, and
start trying to merge this into a bisectable patchset.


BR,
-R

>
> We could do a first pass patch that adds the var =
> instr->variables[0]; and removes
> the instr->variables[0] references elsewhere.
>
> Up to you though, not sure how much extra work it would generate.
>
> Also for Jason, strct is less chars than struct_type, but I expect the
> number of times you typo it might end up more keystrokes :-P
>
> For the series:
> Reviewed-by: Dave Airlie 
>
> Dave.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/19] AMD support for NIR deref instructions.

[Dave Airlie]

On 13 May 2018 at 10:19, Bas Nieuwenhuizen  wrote:
> This implements support in radv and radeonsi for NIR deref
> instructions instead of deref chains.
>
> It contains 4 parts:
> - patch 1 is a fixup for the initial series by Jason
> - Add support everywhere for instruction based derefs
> - Stop lowering them to deref chains (may need to be synchronized
>   with other drivers and core nir code to keep things bisectable)
> - Remove support for deref chains.

Okay I've read over this and I'm pretty happy with it.

I do wonder if we could lower the impact on some of the code by
adding the wrappers functions and getting var in a few places.

i.e. instead of
+  nir_variable *var = uses_deref_chain ? instr->variables[0]->var :
+
nir_deref_instr_get_variable(nir_instr_as_deref(instr->src[0].ssa->parent_instr);
-   int idx = instr->variables[0]->var->data.driver_location;
+   int idx = var->data.driver_location;

We could do a first pass patch that adds the var =
instr->variables[0]; and removes
the instr->variables[0] references elsewhere.

Up to you though, not sure how much extra work it would generate.

Also for Jason, strct is less chars than struct_type, but I expect the
number of times you typo it might end up more keystrokes :-P

For the series:
Reviewed-by: Dave Airlie 

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 00/19] AMD support for NIR deref instructions.

[Bas Nieuwenhuizen]

This implements support in radv and radeonsi for NIR deref
instructions instead of deref chains.

It contains 4 parts:
- patch 1 is a fixup for the initial series by Jason
- Add support everywhere for instruction based derefs
- Stop lowering them to deref chains (may need to be synchronized
  with other drivers and core nir code to keep things bisectable)
- Remove support for deref chains.

You can find this series at

https://github.com/bnieuwenhuizen/mesa/commits/derefs-v1


Bas Nieuwenhuizen (19):
  fixup! anv,i965,radv,st,ir3: Call nir_lower_deref_instrs
  ac/nir: Implement the deref instr for shared memory.
  ac/nir: Support deref instructions in get_sampler_desc.
  ac/nir: Support deref instructions in tex instructions.
  ac/nir: Implement derefs for integer gather4 lowering.
  ac/nir: Add deref support to image intrinsics.
  radv: Add shader info support for image deref instructions.
  ac/nir: Add deref based var loads/stores.
  radv: Gather info for deref instr based load/store.
  ac/nir: Add shared atomic deref instr support.
  ac/nir: Add deref interp support.
  radv: Use deref instructions for tex derefs in meta shaders.
  radv: Remove image_var stores.
  radeonsi: Add deref support to the nir scan pass.
  radv: Stop lowering deref instructions.
  st/nir: Stop lowering deref instructions.
  ac/nir: Remove deref chain support.
  radv: Remove deref chain support in radv shader info pass.
  radeonsi: Remove deref chain support in nir scan pass.

 src/amd/common/ac_nir_to_llvm.c  | 547 ++-
 src/amd/vulkan/radv_meta.c   |  20 +-
 src/amd/vulkan/radv_meta_blit.c  |  30 +-
 src/amd/vulkan/radv_meta_blit2d.c|  21 +-
 src/amd/vulkan/radv_meta_bufimage.c  |  62 ++-
 src/amd/vulkan/radv_meta_fast_clear.c|  17 +-
 src/amd/vulkan/radv_meta_resolve_cs.c|  10 +-
 src/amd/vulkan/radv_shader.c |   2 -
 src/amd/vulkan/radv_shader_info.c| 133 ++---
 src/gallium/drivers/radeonsi/si_shader_nir.c |  72 ++-
 src/mesa/state_tracker/st_glsl_to_nir.cpp|   2 -
 11 files changed, 491 insertions(+), 425 deletions(-)

-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev