[Mesa-dev] [PATCH 1/2] nir: Initial implementation of a nir_instr_worklist

[Eric Anholt]

Thomas Helland <thomashelland90 at gmail.com> writes:

> Make a simple worklist by basically just wrapping u_vector.
> This is intended used in nir_opt_dce to reduce the number of calls
> to ralloc, as we are currenlty spamming ralloc quite bad. It should
> also give better cache locality and much lower memory usage.
> ---
>  src/compiler/nir/nir_worklist.h | 69 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>
> diff --git a/src/compiler/nir/nir_worklist.h b/src/compiler/nir/nir_worklist.h
> index 39521a386c..5071c7aec1 100644
> --- a/src/compiler/nir/nir_worklist.h
> +++ b/src/compiler/nir/nir_worklist.h
> @@ -30,6 +30,8 @@
>  #define _NIR_WORKLIST_
>  
>  #include "nir.h"
> +#include "util/set.h"
> +#include "util/u_vector.h"
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -83,6 +85,73 @@ nir_block *nir_block_worklist_peek_tail(const nir_block_worklist *w);
>  
>  nir_block *nir_block_worklist_pop_tail(nir_block_worklist *w);
>  
> +
> +
> +

Drop two of these extra newlines?

Also, the "typedef struct nir_instr_worklist_node
nir_instr_worklist_node;" in the second patch should probably be in this
one.

> +/*
> + * This worklist implementation, in contrast to the block worklist, does not
> + * have unique entries, meaning a nir_instr can be inserted more than once
> + * into the worklist. It uses u_vector to keep the overhead and memory
> + * footprint at a minimum.
> + *

Trailing whitespace here.

Other than that, this looks great.  I particularly like that you've
explained the choice of the implementation details in the comments here.

With these little nits fixed, both patches are:

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180320/f896b642/attachment-0001.sig>