Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Caio Marcelo de Oliveira Filho]

> I've skipped 21 and 28 because I wanted to give a deeper look at the
> originals.

OK. Patches 21 and 28 are

Reviewed-by: Caio Marcelo de Oliveira Filho 

Patch 12 seems a candidate to get in early. (It is R-B'ed in my
previous mail).


Thanks,
Caio

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

Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Jason Ekstrand]

On Mon, Apr 9, 2018 at 6:20 PM, Jason Ekstrand  wrote:

> On Mon, Apr 9, 2018 at 4:58 PM, Caio Marcelo de Oliveira Filho <
> caio.olive...@intel.com> wrote:
>
>> Hi,
>>
>> Given the fixes you already made based on my comments. Patches 1-20,
>> 22-27, 29-43, and 61 (multiview!) are
>>
>> Reviewed-by: Caio Marcelo de Oliveira Filho 
>>
>> Patches 46-47 and 49 seem to be valid regardless the rest of the code,
>> so I'd consider getting them in independently. They are also R-b'ed.
>>
>
Good call.  I'll go ahead and land those once my Jenkins run completes.

--Jason


> Thanks!
>
>
>> I've skipped 21 and 28 because I wanted to give a deeper look at the
>> originals.
>>
>> From the perspective of someone that is living with deref_vars for
>> just a short time, I like the idea of removing one special
>> construction (derefs) and rely on instructions instead.
>>
>> Which made me wonder: was there a special factor that led NIR to start
>> with the "old-school derefs" in the first place? Other day Curro asked
>> about one of the "selling points" of NIR being it did not have all
>> those nodes representing dereferences. I digged up an old comment to
>> what I think he was referring to
>>
>> https://lists.freedesktop.org/archives/mesa-dev/2014-Februar
>> y/053477.html
>>
>> - All the ir_dereference chains blow up the memory usage, and the
>> constant pointer chasing in the recursive algorithms needed to handle
>> them is not just cache-unfriendly but "cache-mean."
>>
>> How does deref_instructions avoid being "cache-mean" as their
>> "predecessors"? Was the blow up more a result of how the instructions
>> were structured than the fact it had those dereferences nodes?
>>
>
> The blow up was mostly due to the fact that GLSL IR uses dereferences for
> *everything*.  NIR, in contrast, uses SSA defs for 95% of all temporary
> values so there simply aren't as many deref chains in play.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Jason Ekstrand]

On Mon, Apr 9, 2018 at 4:58 PM, Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:

> Hi,
>
> Given the fixes you already made based on my comments. Patches 1-20,
> 22-27, 29-43, and 61 (multiview!) are
>
> Reviewed-by: Caio Marcelo de Oliveira Filho 
>
> Patches 46-47 and 49 seem to be valid regardless the rest of the code,
> so I'd consider getting them in independently. They are also R-b'ed.
>

Thanks!


> I've skipped 21 and 28 because I wanted to give a deeper look at the
> originals.
>
> From the perspective of someone that is living with deref_vars for
> just a short time, I like the idea of removing one special
> construction (derefs) and rely on instructions instead.
>
> Which made me wonder: was there a special factor that led NIR to start
> with the "old-school derefs" in the first place? Other day Curro asked
> about one of the "selling points" of NIR being it did not have all
> those nodes representing dereferences. I digged up an old comment to
> what I think he was referring to
>
> https://lists.freedesktop.org/archives/mesa-dev/2014-
> February/053477.html
>
> - All the ir_dereference chains blow up the memory usage, and the
> constant pointer chasing in the recursive algorithms needed to handle
> them is not just cache-unfriendly but "cache-mean."
>
> How does deref_instructions avoid being "cache-mean" as their
> "predecessors"? Was the blow up more a result of how the instructions
> were structured than the fact it had those dereferences nodes?
>

The blow up was mostly due to the fact that GLSL IR uses dereferences for
*everything*.  NIR, in contrast, uses SSA defs for 95% of all temporary
values so there simply aren't as many deref chains in play.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Caio Marcelo de Oliveira Filho]

Hi,

Given the fixes you already made based on my comments. Patches 1-20,
22-27, 29-43, and 61 (multiview!) are

Reviewed-by: Caio Marcelo de Oliveira Filho 

Patches 46-47 and 49 seem to be valid regardless the rest of the code,
so I'd consider getting them in independently. They are also R-b'ed.

I've skipped 21 and 28 because I wanted to give a deeper look at the
originals.

From the perspective of someone that is living with deref_vars for
just a short time, I like the idea of removing one special
construction (derefs) and rely on instructions instead.

Which made me wonder: was there a special factor that led NIR to start
with the "old-school derefs" in the first place? Other day Curro asked
about one of the "selling points" of NIR being it did not have all
those nodes representing dereferences. I digged up an old comment to
what I think he was referring to

https://lists.freedesktop.org/archives/mesa-dev/2014-February/053477.html

- All the ir_dereference chains blow up the memory usage, and the
constant pointer chasing in the recursive algorithms needed to handle
them is not just cache-unfriendly but "cache-mean."

How does deref_instructions avoid being "cache-mean" as their
"predecessors"? Was the blow up more a result of how the instructions
were structured than the fact it had those dereferences nodes?


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

Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Rob Clark]

On Tue, Apr 3, 2018 at 2:32 PM, Jason Ekstrand  wrote:
> This is something that Connor and I have been talking about for some time
> now.  The basic idea is to replace the current singly linked nir_deref list
> with deref instructions.  This is similar to what LLVM does and it offers
> quite a bit more freedom when we start getting more realistic pointers from
> compute applications.
>
> This series implements a complete conversion for both i965 and anv.  As can
> be seen in the penultimate patch, there are three core NIR passes remaining
> to be converted.  None of those three passes is used by any of the Intel
> drivers so I have no ability to test them.  The final patch deletes support
> for old-school deref chains from NIR entirely.  The only NIR-using drivers
> which build with that patch are i965 and anv but it shows that the
> conversion for those two is complete and has also been very useful in
> finding things I missed the first time around.
>
> Somehow, this series manages to shave off 700 lines of code but I wouldn't
> take that to mean much.  Some of that is whole-sale deleting lower_io_types
> (170 lines).  Some of it is that deref instructions and the new function
> call mechanism are more efficient from a data structure perspective because
> you don't have deref chains attached to texture ops and intrinsics.  I've
> also been modernizing as I go and converting some things to use nir_builder
> instead of building instructions manually.  The ammount that deref
> instructions make things easier over deref chains is totally a wash.
>
> Clearly, this can't really proceed until other drivers have added the bits
> (which should be small at this point) to do the conversion.  Someone also
> needs to add "Support deref instructions in..." and "Remove deref chain
> support from..." patches for the three remaining core NIR passes.
>
> My next plan is to try and start experimenting with more advanced
> load/store elimination on shared variables and maybe even SSBOs.  This will
> require properly handling barriers and, thanks to Vulkan's pointer support,
> cast derefs where the source may have come from a phi node or variable.
>
> This series can be found as a branch on gitlab:
>
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-deref-instr-v3
>

Fwiw, I've converted the remaining passes that mesa/st uses (but
i965/anv do not), and things seem to be generally working(ish).  I
still have a piglit run going, I'm sure there are some little things
to fix here/there.

The nir_lower_samplers_as_deref isn't so well tested, since freedreno
doesn't use it.  I hacked up ir3_cmdline compiler with a call to that
pass with nir_print_shader before/after, and the results looked sane.
But not tested at all on real hw.

  https://github.com/freedreno/mesa/commits/nir-deref-instr-v3

(there was one unrelated patch to avoid rebasing jason's -v3 branch
and one to make ir3_cmdline compiler work so I could test-drive
nir_lower_samplers_as_deref)

I've also not tried to make this bisectable yet.  But I guess it
should be enough for others to start testing/converting their drivers.

BR,
-R

> Cc: Rob Clark 
> Cc: Timothy Arceri 
> Cc: Eric Anholt 
> Cc: Connor Abbott 
> Cc: Bas Nieuwenhuizen 
> Cc: Karol Herbst 
>
> Jason Ekstrand (104):
>   nir/validate: Rework intrinsic type validation
>   nir: Add a deref instruction type
>   nir/builder: Add deref building helpers
>   nir: Add _deref versions of all of the _var intrinsics
>   nir: Add deref sources to texture instructions
>   nir: Add helpers for working with deref instructions
>   anv,i965,radv,st,ir3: Call nir_lower_deref_instrs
>   glsl/nir: Only claim to handle intrinsic functions
>   glsl/nir: Use deref instructions instead of dref chains
>   prog/nir: Simplify some load/store operations
>   prog/nir: Use deref instructions for params
>   nir/lower_atomics: Rework the main walker loop a bit
>   nir: Support deref instructions in remove_dead_variables
>   nir: Add a pass for fixing deref modes
>   nir: Support deref instructions in lower_global_vars_to_local
>   nir: Use nir_builder in lower_io_to_temporaries
>   nir: Support deref instructions in lower_io_to_temporaries
>   nir: Add a deref path helper struct
>   nir: Support deref instructions in lower_var_copies
>   nir: Support deref instructions in split_var_copies
>   nir: Support deref instructions in lower_vars_to_ssa
>   nir: Support deref instructions in lower_indirect_derefs
>   nir/deref: Add a deref cleanup function
>   nir: Support deref instructions in lower_system_values
>   nir: Support deref instructions in lower_clip_cull
>   nir: Support deref instructions in propagate_invariant
>   nir: Support deref instructions in gather_info
>   nir: Support deref instructions in lower_io
>   nir: Support deref instructions in lower_atomics
>   nir: 

Re: [Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Jason Ekstrand]

I ran shader-db on the whole series and this is what I got on KBL:

total instructions in shared programs: 15201981 -> 15231264 (0.19%)
instructions in affected programs: 315139 -> 344422 (9.29%)
helped: 33
HURT: 1253

This should by and large be a no-op but clearly something is amiss.  I'll
take a look later.



On Tue, Apr 3, 2018 at 11:32 AM, Jason Ekstrand 
wrote:

> This is something that Connor and I have been talking about for some time
> now.  The basic idea is to replace the current singly linked nir_deref list
> with deref instructions.  This is similar to what LLVM does and it offers
> quite a bit more freedom when we start getting more realistic pointers from
> compute applications.
>
> This series implements a complete conversion for both i965 and anv.  As can
> be seen in the penultimate patch, there are three core NIR passes remaining
> to be converted.  None of those three passes is used by any of the Intel
> drivers so I have no ability to test them.  The final patch deletes support
> for old-school deref chains from NIR entirely.  The only NIR-using drivers
> which build with that patch are i965 and anv but it shows that the
> conversion for those two is complete and has also been very useful in
> finding things I missed the first time around.
>
> Somehow, this series manages to shave off 700 lines of code but I wouldn't
> take that to mean much.  Some of that is whole-sale deleting lower_io_types
> (170 lines).  Some of it is that deref instructions and the new function
> call mechanism are more efficient from a data structure perspective because
> you don't have deref chains attached to texture ops and intrinsics.  I've
> also been modernizing as I go and converting some things to use nir_builder
> instead of building instructions manually.  The ammount that deref
> instructions make things easier over deref chains is totally a wash.
>
> Clearly, this can't really proceed until other drivers have added the bits
> (which should be small at this point) to do the conversion.  Someone also
> needs to add "Support deref instructions in..." and "Remove deref chain
> support from..." patches for the three remaining core NIR passes.
>
> My next plan is to try and start experimenting with more advanced
> load/store elimination on shared variables and maybe even SSBOs.  This will
> require properly handling barriers and, thanks to Vulkan's pointer support,
> cast derefs where the source may have come from a phi node or variable.
>
> This series can be found as a branch on gitlab:
>
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/
> review/nir-deref-instr-v3
>
> Cc: Rob Clark 
> Cc: Timothy Arceri 
> Cc: Eric Anholt 
> Cc: Connor Abbott 
> Cc: Bas Nieuwenhuizen 
> Cc: Karol Herbst 
>
> Jason Ekstrand (104):
>   nir/validate: Rework intrinsic type validation
>   nir: Add a deref instruction type
>   nir/builder: Add deref building helpers
>   nir: Add _deref versions of all of the _var intrinsics
>   nir: Add deref sources to texture instructions
>   nir: Add helpers for working with deref instructions
>   anv,i965,radv,st,ir3: Call nir_lower_deref_instrs
>   glsl/nir: Only claim to handle intrinsic functions
>   glsl/nir: Use deref instructions instead of dref chains
>   prog/nir: Simplify some load/store operations
>   prog/nir: Use deref instructions for params
>   nir/lower_atomics: Rework the main walker loop a bit
>   nir: Support deref instructions in remove_dead_variables
>   nir: Add a pass for fixing deref modes
>   nir: Support deref instructions in lower_global_vars_to_local
>   nir: Use nir_builder in lower_io_to_temporaries
>   nir: Support deref instructions in lower_io_to_temporaries
>   nir: Add a deref path helper struct
>   nir: Support deref instructions in lower_var_copies
>   nir: Support deref instructions in split_var_copies
>   nir: Support deref instructions in lower_vars_to_ssa
>   nir: Support deref instructions in lower_indirect_derefs
>   nir/deref: Add a deref cleanup function
>   nir: Support deref instructions in lower_system_values
>   nir: Support deref instructions in lower_clip_cull
>   nir: Support deref instructions in propagate_invariant
>   nir: Support deref instructions in gather_info
>   nir: Support deref instructions in lower_io
>   nir: Support deref instructions in lower_atomics
>   nir: Support deref instructions in lower_wpos_ytransform
>   nir: Support deref instructions in lower_pos_center
>   nir: Support deref instructions in remove_unused_varyings
>   nir: Support deref instructions in loop_analyze
>   nir: Support deref instructions in lower_alpha_test
>   nir: Support deref instructions in lower_clamp_color_outputs
>   nir: Support deref instructions in lower_drawpixels
>   nir: Consider deref instructions in lower_phis_to_scalar
>   nir: Consider deref instructions in 

[Mesa-dev] [PATCH v3 000/104] nir: Move to using instructions for derefs

[Jason Ekstrand]

This is something that Connor and I have been talking about for some time
now.  The basic idea is to replace the current singly linked nir_deref list
with deref instructions.  This is similar to what LLVM does and it offers
quite a bit more freedom when we start getting more realistic pointers from
compute applications.

This series implements a complete conversion for both i965 and anv.  As can
be seen in the penultimate patch, there are three core NIR passes remaining
to be converted.  None of those three passes is used by any of the Intel
drivers so I have no ability to test them.  The final patch deletes support
for old-school deref chains from NIR entirely.  The only NIR-using drivers
which build with that patch are i965 and anv but it shows that the
conversion for those two is complete and has also been very useful in
finding things I missed the first time around.

Somehow, this series manages to shave off 700 lines of code but I wouldn't
take that to mean much.  Some of that is whole-sale deleting lower_io_types
(170 lines).  Some of it is that deref instructions and the new function
call mechanism are more efficient from a data structure perspective because
you don't have deref chains attached to texture ops and intrinsics.  I've
also been modernizing as I go and converting some things to use nir_builder
instead of building instructions manually.  The ammount that deref
instructions make things easier over deref chains is totally a wash.

Clearly, this can't really proceed until other drivers have added the bits
(which should be small at this point) to do the conversion.  Someone also
needs to add "Support deref instructions in..." and "Remove deref chain
support from..." patches for the three remaining core NIR passes.

My next plan is to try and start experimenting with more advanced
load/store elimination on shared variables and maybe even SSBOs.  This will
require properly handling barriers and, thanks to Vulkan's pointer support,
cast derefs where the source may have come from a phi node or variable.

This series can be found as a branch on gitlab:

https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-deref-instr-v3

Cc: Rob Clark 
Cc: Timothy Arceri 
Cc: Eric Anholt 
Cc: Connor Abbott 
Cc: Bas Nieuwenhuizen 
Cc: Karol Herbst 

Jason Ekstrand (104):
  nir/validate: Rework intrinsic type validation
  nir: Add a deref instruction type
  nir/builder: Add deref building helpers
  nir: Add _deref versions of all of the _var intrinsics
  nir: Add deref sources to texture instructions
  nir: Add helpers for working with deref instructions
  anv,i965,radv,st,ir3: Call nir_lower_deref_instrs
  glsl/nir: Only claim to handle intrinsic functions
  glsl/nir: Use deref instructions instead of dref chains
  prog/nir: Simplify some load/store operations
  prog/nir: Use deref instructions for params
  nir/lower_atomics: Rework the main walker loop a bit
  nir: Support deref instructions in remove_dead_variables
  nir: Add a pass for fixing deref modes
  nir: Support deref instructions in lower_global_vars_to_local
  nir: Use nir_builder in lower_io_to_temporaries
  nir: Support deref instructions in lower_io_to_temporaries
  nir: Add a deref path helper struct
  nir: Support deref instructions in lower_var_copies
  nir: Support deref instructions in split_var_copies
  nir: Support deref instructions in lower_vars_to_ssa
  nir: Support deref instructions in lower_indirect_derefs
  nir/deref: Add a deref cleanup function
  nir: Support deref instructions in lower_system_values
  nir: Support deref instructions in lower_clip_cull
  nir: Support deref instructions in propagate_invariant
  nir: Support deref instructions in gather_info
  nir: Support deref instructions in lower_io
  nir: Support deref instructions in lower_atomics
  nir: Support deref instructions in lower_wpos_ytransform
  nir: Support deref instructions in lower_pos_center
  nir: Support deref instructions in remove_unused_varyings
  nir: Support deref instructions in loop_analyze
  nir: Support deref instructions in lower_alpha_test
  nir: Support deref instructions in lower_clamp_color_outputs
  nir: Support deref instructions in lower_drawpixels
  nir: Consider deref instructions in lower_phis_to_scalar
  nir: Consider deref instructions in opt_peephole_select
  nir: Support deref instructions in opt_undef
  intel,ir3: Disable nir_opt_copy_prop_vars
  intel/nir: Fixup deref modes after lowering patch vertices
  i965: Move nir_lower_deref_instrs to right before locals_to_regs
  st/nir: Move lower_deref_instrs later
  spirv: Use deref instructions for most variables
  nir: Add a concept of per-member structs and a lowering pass
  nir/lower_system_values: Support SYSTEM_VALUE_LOCAL_GROUP_SIZE
  spirv: Use the LOCAL_GROUP_SIZE system value
  nir/spirv: Pass nir_variable_data into apply_var_decoration