[Mesa-dev] [PATCH 1/7] glsl: Separate overlapping sentinel nodes in exec_list.

Ian Romanick idr at freedesktop.org
Fri Jul 1 22:16:26 UTC 2016


On 06/27/2016 02:42 PM, Matt Turner wrote:
> I do appreciate the cleverness, but unfortunately it prevents a lot more
> cleverness in the form of additional compiler optimizations brought on
> by -fstrict-aliasing.
> 
> No difference in OglBatch7 (n=20).
> 
> Co-authored-by: Davin McCall <davmac at davmac.org>
> ---
>  src/compiler/glsl/ast.h                            |   4 +-
>  src/compiler/glsl/ast_function.cpp                 |  22 ++--
>  src/compiler/glsl/ast_to_hir.cpp                   |   6 +-
>  src/compiler/glsl/ast_type.cpp                     |   2 +-
>  src/compiler/glsl/glsl_parser_extras.cpp           |   6 +-
>  src/compiler/glsl/ir.cpp                           |   8 +-
>  src/compiler/glsl/ir_clone.cpp                     |   2 +-
>  src/compiler/glsl/ir_constant_expression.cpp       |   2 +-
>  src/compiler/glsl/ir_function.cpp                  |  14 +--
>  src/compiler/glsl/ir_reader.cpp                    |   4 +-
>  src/compiler/glsl/ir_validate.cpp                  |   4 +-
>  src/compiler/glsl/list.h                           | 136 +++++++++------------
>  src/compiler/glsl/lower_distance.cpp               |   4 +-
>  src/compiler/glsl/lower_jumps.cpp                  |   2 +-
>  src/compiler/glsl/lower_packed_varyings.cpp        |   8 +-
>  src/compiler/glsl/lower_tess_level.cpp             |   4 +-
>  src/compiler/glsl/opt_conditional_discard.cpp      |   6 +-
>  src/compiler/glsl/opt_dead_builtin_varyings.cpp    |   2 +-
>  src/compiler/glsl/opt_dead_code.cpp                |   2 +-
>  src/compiler/glsl/opt_flatten_nested_if_blocks.cpp |   2 +-
>  src/compiler/nir/nir.h                             |   4 +-
>  src/compiler/nir/nir_opt_gcm.c                     |   2 +-
>  src/mesa/drivers/dri/i965/brw_cfg.h                |   2 +-
>  src/mesa/drivers/dri/i965/brw_fs_builder.h         |   2 +-
>  src/mesa/drivers/dri/i965/brw_vec4_builder.h       |   2 +-
>  25 files changed, 116 insertions(+), 136 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index 06c7b03..5029ba7 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -346,8 +346,8 @@ public:
>  
>     bool is_single_dimension() const
>     {
> -      return this->array_dimensions.tail_pred->prev != NULL &&
> -             this->array_dimensions.tail_pred->prev->is_head_sentinel();
> +      return this->array_dimensions.tail_sentinel.prev->prev != NULL &&
> +             this->array_dimensions.tail_sentinel.prev->prev->is_head_sentinel();
>     }
>  
>     virtual void print(void) const;
> diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
> index f74394f..41b0765 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -186,8 +186,8 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
>  		       exec_list &actual_ir_parameters,
>  		       exec_list &actual_ast_parameters)
>  {
> -   exec_node *actual_ir_node  = actual_ir_parameters.head;
> -   exec_node *actual_ast_node = actual_ast_parameters.head;
> +   exec_node *actual_ir_node  = actual_ir_parameters.head_sentinel.next;
> +   exec_node *actual_ast_node = actual_ast_parameters.head_sentinel.next;

This seems pretty annoying.  I've had this reply sitting open since
Tuesday morning... and I don't have a good solution.

Normally you'd add a function or macro to wrap this, but we already have
exec_list::get_head.  That function has the extra semantic that if the
list is empty, you get NULL.  Most of the places that don't already use
that method don't want to use it.  They either already know that the
list is not empty (as a precondition) or they're going to compare the
node pointer with the sentinel.

We could add another function... but what do you call it, and how do you
keep people from using the wrong one?  get_head_raw()?  get_head_fast()?
_head()?  first()?   Hmm... maybe change the existing functions to
get_first_node() / get_last_node() and call the new functions get_head()
/ get_tail()?

All I know is that I really don't want to type head_sentinel.next or
tail_sentinel.prev... mostly because at least once I'll accidentally
type (or cut-and-paste, or search-and-replace) tail_sentinel.next, and
I'll never find the bug.

Anyway... patches 2/7 are

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

I'd like to have some kind of improvement here, but I think the
mechanics of the change are correct.

>     foreach_in_list(const ir_variable, formal, &sig->parameters) {
>        /* The lists must be the same length. */
> @@ -318,10 +318,12 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
>     const char *func_name = sig->function_name();
>     bool is_atomic = is_atomic_function(func_name);
>     if (is_atomic) {
> -      const ir_rvalue *const actual = (ir_rvalue *) actual_ir_parameters.head;
> +      const ir_rvalue *const actual =
> +         (ir_rvalue *) actual_ir_parameters.head_sentinel.next;
>  
>        const ast_expression *const actual_ast =
> -         exec_node_data(ast_expression, actual_ast_parameters.head, link);
> +         exec_node_data(ast_expression,
> +                        actual_ast_parameters.head_sentinel.next, link);
>        YYLTYPE loc = actual_ast->get_location();
>  
>        if (!verify_first_atomic_parameter(&loc, state,
> @@ -1176,7 +1178,7 @@ constant_record_constructor(const glsl_type *constructor_type,
>  bool
>  single_scalar_parameter(exec_list *parameters)
>  {
> -   const ir_rvalue *const p = (ir_rvalue *) parameters->head;
> +   const ir_rvalue *const p = (ir_rvalue *) parameters->head_sentinel.next;
>     assert(((ir_rvalue *)p)->as_rvalue() != NULL);
>  
>     return (p->type->is_scalar() && p->next->is_tail_sentinel());
> @@ -1220,7 +1222,7 @@ emit_inline_vector_constructor(const glsl_type *type,
>      */
>     const unsigned lhs_components = type->components();
>     if (single_scalar_parameter(parameters)) {
> -      ir_rvalue *first_param = (ir_rvalue *)parameters->head;
> +      ir_rvalue *first_param = (ir_rvalue *)parameters->head_sentinel.next;
>        ir_rvalue *rhs = new(ctx) ir_swizzle(first_param, 0, 0, 0, 0,
>  					   lhs_components);
>        ir_dereference_variable *lhs = new(ctx) ir_dereference_variable(var);
> @@ -1421,7 +1423,7 @@ emit_inline_matrix_constructor(const glsl_type *type,
>      *    to the upper left portion of the constructed matrix, and the remaining
>      *    elements take values from the identity matrix.
>      */
> -   ir_rvalue *const first_param = (ir_rvalue *) parameters->head;
> +   ir_rvalue *const first_param = (ir_rvalue *) parameters->head_sentinel.next;
>     if (single_scalar_parameter(parameters)) {
>        /* Assign the scalar to the X component of a vec4, and fill the remaining
>         * components with zero.
> @@ -1673,7 +1675,7 @@ emit_inline_record_constructor(const glsl_type *type,
>  
>     instructions->push_tail(var);
>  
> -   exec_node *node = parameters->head;
> +   exec_node *node = parameters->head_sentinel.next;
>     for (unsigned i = 0; i < type->length; i++) {
>        assert(!node->is_tail_sentinel());
>  
> @@ -1706,7 +1708,7 @@ process_record_constructor(exec_list *instructions,
>     process_parameters(instructions, &actual_parameters,
>                        parameters, state);
>  
> -   exec_node *node = actual_parameters.head;
> +   exec_node *node = actual_parameters.head_sentinel.next;
>     for (unsigned i = 0; i < constructor_type->length; i++) {
>        ir_rvalue *ir = (ir_rvalue *) node;
>  
> @@ -2042,7 +2044,7 @@ ast_function_expression::hir(exec_list *instructions,
>        if (all_parameters_are_constant) {
>  	 return new(ctx) ir_constant(constructor_type, &actual_parameters);
>        } else if (constructor_type->is_scalar()) {
> -	 return dereference_component((ir_rvalue *) actual_parameters.head,
> +         return dereference_component((ir_rvalue *) actual_parameters.head_sentinel.next,
>  				      0);
>        } else if (constructor_type->is_vector()) {
>  	 return emit_inline_vector_constructor(constructor_type,
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index 7da734c..70393c8 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -2002,7 +2002,7 @@ ast_expression::do_hir(exec_list *instructions,
>            * effect, but I don't think these cases exist in GLSL.  Either way,
>            * it would be a giant hassle to replicate that behavior.
>            */
> -         if (previous_tail_pred == instructions->tail_pred) {
> +         if (previous_tail_pred == instructions->tail_sentinel.prev) {
>              _mesa_glsl_warning(&previous_operand_loc, state,
>                                 "left-hand operand of comma expression has "
>                                 "no effect");
> @@ -2013,7 +2013,7 @@ ast_expression::do_hir(exec_list *instructions,
>            * return NULL when the list is empty.  We don't care about that
>            * here, so using tail_pred directly is fine.
>            */
> -         previous_tail_pred = instructions->tail_pred;
> +         previous_tail_pred = instructions->tail_sentinel.prev;
>           previous_operand_loc = ast->get_location();
>  
>           result = ast->hir(instructions, state);
> @@ -2238,7 +2238,7 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
>           }
>        }
>  
> -      for (exec_node *node = array_specifier->array_dimensions.tail_pred;
> +      for (exec_node *node = array_specifier->array_dimensions.tail_sentinel.prev;
>             !node->is_head_sentinel(); node = node->prev) {
>           unsigned array_size = process_array_size(node, state);
>           array_type = glsl_type::get_array_instance(array_type, array_size);
> diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
> index 1372ff7..1b13e98 100644
> --- a/src/compiler/glsl/ast_type.cpp
> +++ b/src/compiler/glsl/ast_type.cpp
> @@ -688,7 +688,7 @@ ast_layout_expression::process_qualifier_constant(struct _mesa_glsl_parse_state
>     if (!can_be_zero)
>        min_value = 1;
>  
> -   for (exec_node *node = layout_const_expressions.head;
> +   for (exec_node *node = layout_const_expressions.head_sentinel.next;
>             !node->is_tail_sentinel(); node = node->next) {
>  
>        exec_list dummy_instructions;
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index 09f7477..8b643e1 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -831,7 +831,7 @@ _mesa_ast_set_aggregate_type(const glsl_type *type,
>         * E.g., if <type> if struct S[2] we want to set each element's type to
>         * struct S.
>         */
> -      for (exec_node *expr_node = ai->expressions.head;
> +      for (exec_node *expr_node = ai->expressions.head_sentinel.next;
>             !expr_node->is_tail_sentinel();
>             expr_node = expr_node->next) {
>           ast_expression *expr = exec_node_data(ast_expression, expr_node,
> @@ -843,7 +843,7 @@ _mesa_ast_set_aggregate_type(const glsl_type *type,
>  
>     /* If the aggregate is a struct, recursively set its fields' types. */
>     } else if (type->is_record()) {
> -      exec_node *expr_node = ai->expressions.head;
> +      exec_node *expr_node = ai->expressions.head_sentinel.next;
>  
>        /* Iterate through the struct's fields. */
>        for (unsigned i = 0; !expr_node->is_tail_sentinel() && i < type->length;
> @@ -857,7 +857,7 @@ _mesa_ast_set_aggregate_type(const glsl_type *type,
>        }
>     /* If the aggregate is a matrix, set its columns' types. */
>     } else if (type->is_matrix()) {
> -      for (exec_node *expr_node = ai->expressions.head;
> +      for (exec_node *expr_node = ai->expressions.head_sentinel.next;
>             !expr_node->is_tail_sentinel();
>             expr_node = expr_node->next) {
>           ast_expression *expr = exec_node_data(ast_expression, expr_node,
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 7961f00..61b42f8 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -807,7 +807,7 @@ ir_constant::ir_constant(const struct glsl_type *type, exec_list *value_list)
>        this->value.u[i] = 0;
>     }
>  
> -   ir_constant *value = (ir_constant *) (value_list->head);
> +   ir_constant *value = (ir_constant *) (value_list->head_sentinel.next);
>  
>     /* Constructors with exactly one scalar argument are special for vectors
>      * and matrices.  For vectors, the scalar value is replicated to fill all
> @@ -1073,7 +1073,7 @@ ir_constant::get_record_field(const char *name)
>     if (this->components.is_empty())
>        return NULL;
>  
> -   exec_node *node = this->components.head;
> +   exec_node *node = this->components.head_sentinel.next;
>     for (int i = 0; i < idx; i++) {
>        node = node->next;
>  
> @@ -1197,8 +1197,8 @@ ir_constant::has_value(const ir_constant *c) const
>     }
>  
>     if (this->type->base_type == GLSL_TYPE_STRUCT) {
> -      const exec_node *a_node = this->components.head;
> -      const exec_node *b_node = c->components.head;
> +      const exec_node *a_node = this->components.head_sentinel.next;
> +      const exec_node *b_node = c->components.head_sentinel.next;
>  
>        while (!a_node->is_tail_sentinel()) {
>  	 assert(!b_node->is_tail_sentinel());
> diff --git a/src/compiler/glsl/ir_clone.cpp b/src/compiler/glsl/ir_clone.cpp
> index 60d1526..e44ce2f 100644
> --- a/src/compiler/glsl/ir_clone.cpp
> +++ b/src/compiler/glsl/ir_clone.cpp
> @@ -344,7 +344,7 @@ ir_constant::clone(void *mem_ctx, struct hash_table *ht) const
>        ir_constant *c = new(mem_ctx) ir_constant;
>  
>        c->type = this->type;
> -      for (exec_node *node = this->components.head
> +      for (exec_node *node = this->components.head_sentinel.next
>  	      ; !node->is_tail_sentinel()
>  	      ; node = node->next) {
>  	 ir_constant *const orig = (ir_constant *) node;
> diff --git a/src/compiler/glsl/ir_constant_expression.cpp b/src/compiler/glsl/ir_constant_expression.cpp
> index d961aa9..e79d5ef 100644
> --- a/src/compiler/glsl/ir_constant_expression.cpp
> +++ b/src/compiler/glsl/ir_constant_expression.cpp
> @@ -2062,7 +2062,7 @@ ir_function_signature::constant_expression_value(exec_list *actual_parameters, s
>      * have to use the variable objects from the object with the body,
>      * but the parameter instanciation on the current object.
>      */
> -   const exec_node *parameter_info = origin ? origin->parameters.head : parameters.head;
> +   const exec_node *parameter_info = origin ? origin->parameters.head_sentinel.next : parameters.head_sentinel.next;
>  
>     foreach_in_list(ir_rvalue, n, actual_parameters) {
>        ir_constant *constant = n->constant_expression_value(variable_context);
> diff --git a/src/compiler/glsl/ir_function.cpp b/src/compiler/glsl/ir_function.cpp
> index 0b4cb4b..d4d0a59 100644
> --- a/src/compiler/glsl/ir_function.cpp
> +++ b/src/compiler/glsl/ir_function.cpp
> @@ -43,8 +43,8 @@ static parameter_list_match_t
>  parameter_lists_match(_mesa_glsl_parse_state *state,
>                        const exec_list *list_a, const exec_list *list_b)
>  {
> -   const exec_node *node_a = list_a->head;
> -   const exec_node *node_b = list_b->head;
> +   const exec_node *node_a = list_a->head_sentinel.next;
> +   const exec_node *node_b = list_b->head_sentinel.next;
>  
>     /* This is set to true if there is an inexact match requiring an implicit
>      * conversion. */
> @@ -222,9 +222,9 @@ is_best_inexact_overload(const exec_list *actual_parameters,
>        if (*other == sig)
>           continue;
>  
> -      const exec_node *node_a = sig->parameters.head;
> -      const exec_node *node_b = (*other)->parameters.head;
> -      const exec_node *node_p = actual_parameters->head;
> +      const exec_node *node_a = sig->parameters.head_sentinel.next;
> +      const exec_node *node_b = (*other)->parameters.head_sentinel.next;
> +      const exec_node *node_p = actual_parameters->head_sentinel.next;
>  
>        bool better_for_some_parameter = false;
>  
> @@ -366,8 +366,8 @@ ir_function::matching_signature(_mesa_glsl_parse_state *state,
>  static bool
>  parameter_lists_match_exact(const exec_list *list_a, const exec_list *list_b)
>  {
> -   const exec_node *node_a = list_a->head;
> -   const exec_node *node_b = list_b->head;
> +   const exec_node *node_a = list_a->head_sentinel.next;
> +   const exec_node *node_b = list_b->head_sentinel.next;
>  
>     for (/* empty */
>  	; !node_a->is_tail_sentinel() && !node_b->is_tail_sentinel()
> diff --git a/src/compiler/glsl/ir_reader.cpp b/src/compiler/glsl/ir_reader.cpp
> index 15315aa..9f933a9 100644
> --- a/src/compiler/glsl/ir_reader.cpp
> +++ b/src/compiler/glsl/ir_reader.cpp
> @@ -208,7 +208,7 @@ ir_reader::read_function(s_expression *expr, bool skip_body)
>     /* Skip over "function" tag and function name (which are guaranteed to be
>      * present by the above PARTIAL_MATCH call).
>      */
> -   exec_node *node = ((s_list *) expr)->subexpressions.head->next->next;
> +   exec_node *node = ((s_list *) expr)->subexpressions.head_sentinel.next->next->next;
>     for (/* nothing */; !node->is_tail_sentinel(); node = node->next) {
>        s_expression *s_sig = (s_expression *) node;
>        read_function_sig(f, s_sig, skip_body);
> @@ -251,7 +251,7 @@ ir_reader::read_function_sig(ir_function *f, s_expression *expr, bool skip_body)
>     state->symbols->push_scope();
>  
>     /* Skip over the "parameters" tag. */
> -   exec_node *node = paramlist->subexpressions.head->next;
> +   exec_node *node = paramlist->subexpressions.head_sentinel.next->next;
>     for (/* nothing */; !node->is_tail_sentinel(); node = node->next) {
>        ir_variable *var = read_declaration((s_expression *) node);
>        if (var == NULL)
> diff --git a/src/compiler/glsl/ir_validate.cpp b/src/compiler/glsl/ir_validate.cpp
> index 126f9bf..ab0d496 100644
> --- a/src/compiler/glsl/ir_validate.cpp
> +++ b/src/compiler/glsl/ir_validate.cpp
> @@ -840,8 +840,8 @@ ir_validate::visit_enter(ir_call *ir)
>        abort();
>     }
>  
> -   const exec_node *formal_param_node = callee->parameters.head;
> -   const exec_node *actual_param_node = ir->actual_parameters.head;
> +   const exec_node *formal_param_node = callee->parameters.head_sentinel.next;
> +   const exec_node *actual_param_node = ir->actual_parameters.head_sentinel.next;
>     while (true) {
>        if (formal_param_node->is_tail_sentinel()
>            != actual_param_node->is_tail_sentinel()) {
> diff --git a/src/compiler/glsl/list.h b/src/compiler/glsl/list.h
> index a1c4d82..a61a9bf 100644
> --- a/src/compiler/glsl/list.h
> +++ b/src/compiler/glsl/list.h
> @@ -32,36 +32,12 @@
>   *
>   * A list is empty if either the head sentinel's \c next pointer points to the
>   * tail sentinel or the tail sentinel's \c prev poiner points to the head
> - * sentinel.
> - *
> - * Instead of tracking two separate \c node structures and a \c list structure
> - * that points to them, the sentinel nodes are in a single structure.  Noting
> - * that each sentinel node always has one \c NULL pointer, the \c NULL
> - * pointers occupy the same memory location.  In the \c list structure
> - * contains a the following:
> - *
> - *   - A \c head pointer that represents the \c next pointer of the
> - *     head sentinel node.
> - *   - A \c tail pointer that represents the \c prev pointer of the head
> - *     sentinel node and the \c next pointer of the tail sentinel node.  This
> - *     pointer is \b always \c NULL.
> - *   - A \c tail_prev pointer that represents the \c prev pointer of the
> - *     tail sentinel node.
> - *
> - * Therefore, if \c head->next is \c NULL or \c tail_prev->prev is \c NULL,
> - * the list is empty.
> + * sentinel. The head sentinel and tail sentinel nodes are allocated within the
> + * list structure.
>   *
>   * Do note that this means that the list nodes will contain pointers into the
>   * list structure itself and as a result you may not \c realloc() an  \c
>   * exec_list or any structure in which an \c exec_list is embedded.
> - *
> - * To anyone familiar with "exec lists" on the Amiga, this structure should
> - * be immediately recognizable.  See the following link for the original Amiga
> - * operating system documentation on the subject.
> - *
> - * http://www.natami.net/dev/Libraries_Manual_guide/node02D7.html
> - *
> - * \author Ian Romanick <ian.d.romanick at intel.com>
>   */
>  
>  #pragma once
> @@ -307,9 +283,8 @@ struct exec_node;
>  #endif
>  
>  struct exec_list {
> -   struct exec_node *head;
> -   struct exec_node *tail;
> -   struct exec_node *tail_pred;
> +   struct exec_node head_sentinel;
> +   struct exec_node tail_sentinel;
>  
>  #ifdef __cplusplus
>     DECLARE_RALLOC_CXX_OPERATORS(exec_list)
> @@ -366,9 +341,10 @@ struct exec_list {
>  static inline void
>  exec_list_make_empty(struct exec_list *list)
>  {
> -   list->head = (struct exec_node *) & list->tail;
> -   list->tail = NULL;
> -   list->tail_pred = (struct exec_node *) & list->head;
> +   list->head_sentinel.next = &list->tail_sentinel;
> +   list->head_sentinel.prev = NULL;
> +   list->tail_sentinel.next = NULL;
> +   list->tail_sentinel.prev = &list->head_sentinel;
>  }
>  
>  static inline bool
> @@ -376,39 +352,39 @@ exec_list_is_empty(const struct exec_list *list)
>  {
>     /* There are three ways to test whether a list is empty or not.
>      *
> -    * - Check to see if the \c head points to the \c tail.
> -    * - Check to see if the \c tail_pred points to the \c head.
> -    * - Check to see if the \c head is the sentinel node by test whether its
> +    * - Check to see if the head sentinel's \c next is the tail sentinel.
> +    * - Check to see if the tail sentinel's \c prev is the head sentinel.
> +    * - Check to see if the head is the sentinel node by test whether its
>      *   \c next pointer is \c NULL.
>      *
>      * The first two methods tend to generate better code on modern systems
>      * because they save a pointer dereference.
>      */
> -   return list->head == (struct exec_node *) &list->tail;
> +   return list->head_sentinel.next == &list->tail_sentinel;
>  }
>  
>  static inline const struct exec_node *
>  exec_list_get_head_const(const struct exec_list *list)
>  {
> -   return !exec_list_is_empty(list) ? list->head : NULL;
> +   return !exec_list_is_empty(list) ? list->head_sentinel.next : NULL;
>  }
>  
>  static inline struct exec_node *
>  exec_list_get_head(struct exec_list *list)
>  {
> -   return !exec_list_is_empty(list) ? list->head : NULL;
> +   return !exec_list_is_empty(list) ? list->head_sentinel.next : NULL;
>  }
>  
>  static inline const struct exec_node *
>  exec_list_get_tail_const(const struct exec_list *list)
>  {
> -   return !exec_list_is_empty(list) ? list->tail_pred : NULL;
> +   return !exec_list_is_empty(list) ? list->tail_sentinel.prev : NULL;
>  }
>  
>  static inline struct exec_node *
>  exec_list_get_tail(struct exec_list *list)
>  {
> -   return !exec_list_is_empty(list) ? list->tail_pred : NULL;
> +   return !exec_list_is_empty(list) ? list->tail_sentinel.prev : NULL;
>  }
>  
>  static inline unsigned
> @@ -417,7 +393,7 @@ exec_list_length(const struct exec_list *list)
>     unsigned size = 0;
>     struct exec_node *node;
>  
> -   for (node = list->head; node->next != NULL; node = node->next) {
> +   for (node = list->head_sentinel.next; node->next != NULL; node = node->next) {
>        size++;
>     }
>  
> @@ -427,21 +403,21 @@ exec_list_length(const struct exec_list *list)
>  static inline void
>  exec_list_push_head(struct exec_list *list, struct exec_node *n)
>  {
> -   n->next = list->head;
> -   n->prev = (struct exec_node *) &list->head;
> +   n->next = list->head_sentinel.next;
> +   n->prev = &list->head_sentinel;
>  
>     n->next->prev = n;
> -   list->head = n;
> +   list->head_sentinel.next = n;
>  }
>  
>  static inline void
>  exec_list_push_tail(struct exec_list *list, struct exec_node *n)
>  {
> -   n->next = (struct exec_node *) &list->tail;
> -   n->prev = list->tail_pred;
> +   n->next = &list->tail_sentinel;
> +   n->prev = list->tail_sentinel.prev;
>  
>     n->prev->next = n;
> -   list->tail_pred = n;
> +   list->tail_sentinel.prev = n;
>  }
>  
>  static inline void
> @@ -449,10 +425,10 @@ exec_list_push_degenerate_list_at_head(struct exec_list *list, struct exec_node
>  {
>     assert(n->prev->next == n);
>  
> -   n->prev->next = list->head;
> -   list->head->prev = n->prev;
> -   n->prev = (struct exec_node *) &list->head;
> -   list->head = n;
> +   n->prev->next = list->head_sentinel.next;
> +   list->head_sentinel.next->prev = n->prev;
> +   n->prev = &list->head_sentinel;
> +   list->head_sentinel.next = n;
>  }
>  
>  static inline struct exec_node *
> @@ -471,12 +447,13 @@ exec_list_move_nodes_to(struct exec_list *list, struct exec_list *target)
>     if (exec_list_is_empty(list)) {
>        exec_list_make_empty(target);
>     } else {
> -      target->head = list->head;
> -      target->tail = NULL;
> -      target->tail_pred = list->tail_pred;
> +      target->head_sentinel.next = list->head_sentinel.next;
> +      target->head_sentinel.prev = NULL;
> +      target->tail_sentinel.next = NULL;
> +      target->tail_sentinel.prev = list->tail_sentinel.prev;
>  
> -      target->head->prev = (struct exec_node *) &target->head;
> -      target->tail_pred->next = (struct exec_node *) &target->tail;
> +      target->head_sentinel.next->prev = &target->head_sentinel;
> +      target->tail_sentinel.prev->next = &target->tail_sentinel;
>  
>        exec_list_make_empty(list);
>     }
> @@ -490,13 +467,13 @@ exec_list_append(struct exec_list *list, struct exec_list *source)
>  
>     /* Link the first node of the source with the last node of the target list.
>      */
> -   list->tail_pred->next = source->head;
> -   source->head->prev = list->tail_pred;
> +   list->tail_sentinel.prev->next = source->head_sentinel.next;
> +   source->head_sentinel.next->prev = list->tail_sentinel.prev;
>  
>     /* Make the tail of the source list be the tail of the target list.
>      */
> -   list->tail_pred = source->tail_pred;
> -   list->tail_pred->next = (struct exec_node *) &list->tail;
> +   list->tail_sentinel.prev = source->tail_sentinel.prev;
> +   list->tail_sentinel.prev->next = &list->tail_sentinel;
>  
>     /* Make the source list empty for good measure.
>      */
> @@ -516,11 +493,11 @@ exec_node_insert_list_before(struct exec_node *n, struct exec_list *before)
>     if (exec_list_is_empty(before))
>        return;
>  
> -   before->tail_pred->next = n;
> -   before->head->prev = n->prev;
> +   before->tail_sentinel.prev->next = n;
> +   before->head_sentinel.next->prev = n->prev;
>  
> -   n->prev->next = before->head;
> -   n->prev = before->tail_pred;
> +   n->prev->next = before->head_sentinel.next;
> +   n->prev = before->tail_sentinel.prev;
>  
>     exec_list_make_empty(before);
>  }
> @@ -530,15 +507,16 @@ exec_list_validate(const struct exec_list *list)
>  {
>     const struct exec_node *node;
>  
> -   assert(list->head->prev == (const struct exec_node *) &list->head);
> -   assert(list->tail == NULL);
> -   assert(list->tail_pred->next == (const struct exec_node *) &list->tail);
> +   assert(list->head_sentinel.next->prev == &list->head_sentinel);
> +   assert(list->head_sentinel.prev == NULL);
> +   assert(list->tail_sentinel.next == NULL);
> +   assert(list->tail_sentinel.prev->next == &list->tail_sentinel);
>  
>     /* We could try to use one of the interators below for this but they all
>      * either require C++ or assume the exec_node is embedded in a structure
>      * which is not the case for this function.
>      */
> -   for (node = list->head; node->next != NULL; node = node->next) {
> +   for (node = list->head_sentinel.next; node->next != NULL; node = node->next) {
>        assert(node->next->prev == node);
>        assert(node->prev->next == node);
>     }
> @@ -622,12 +600,12 @@ inline void exec_node::insert_before(exec_list *before)
>  #endif
>  
>  #define foreach_in_list(__type, __inst, __list)      \
> -   for (__type *(__inst) = (__type *)(__list)->head; \
> +   for (__type *(__inst) = (__type *)(__list)->head_sentinel.next; \
>          !(__inst)->is_tail_sentinel();               \
>          (__inst) = (__type *)(__inst)->next)
>  
>  #define foreach_in_list_reverse(__type, __inst, __list)   \
> -   for (__type *(__inst) = (__type *)(__list)->tail_pred; \
> +   for (__type *(__inst) = (__type *)(__list)->tail_sentinel.prev; \
>          !(__inst)->is_head_sentinel();                    \
>          (__inst) = (__type *)(__inst)->prev)
>  
> @@ -635,20 +613,20 @@ inline void exec_node::insert_before(exec_list *before)
>   * This version is safe even if the current node is removed.
>   */ 
>  #define foreach_in_list_safe(__type, __node, __list) \
> -   for (__type *__node = (__type *)(__list)->head,   \
> +   for (__type *__node = (__type *)(__list)->head_sentinel.next,   \
>                 *__next = (__type *)__node->next;     \
>          __next != NULL;                              \
>          __node = __next, __next = (__type *)__next->next)
>  
>  #define foreach_in_list_reverse_safe(__type, __node, __list) \
> -   for (__type *__node = (__type *)(__list)->tail_pred,      \
> +   for (__type *__node = (__type *)(__list)->tail_sentinel.prev,      \
>                 *__prev = (__type *)__node->prev;             \
>          __prev != NULL;                                      \
>          __node = __prev, __prev = (__type *)__prev->prev)
>  
>  #define foreach_in_list_use_after(__type, __inst, __list) \
>     __type *(__inst);                                      \
> -   for ((__inst) = (__type *)(__list)->head;              \
> +   for ((__inst) = (__type *)(__list)->head_sentinel.next; \
>          !(__inst)->is_tail_sentinel();                    \
>          (__inst) = (__type *)(__inst)->next)
>  /**
> @@ -657,8 +635,8 @@ inline void exec_node::insert_before(exec_list *before)
>   * This is safe against either current node being removed or replaced.
>   */
>  #define foreach_two_lists(__node1, __list1, __node2, __list2) \
> -   for (struct exec_node * __node1 = (__list1)->head,         \
> -                         * __node2 = (__list2)->head,         \
> +   for (struct exec_node * __node1 = (__list1)->head_sentinel.next, \
> +                         * __node2 = (__list2)->head_sentinel.next, \
>                           * __next1 = __node1->next,           \
>                           * __next2 = __node2->next            \
>  	; __next1 != NULL && __next2 != NULL                  \
> @@ -669,19 +647,19 @@ inline void exec_node::insert_before(exec_list *before)
>  
>  #define foreach_list_typed(__type, __node, __field, __list)		\
>     for (__type * __node =						\
> -	   exec_node_data(__type, (__list)->head, __field);		\
> +         exec_node_data(__type, (__list)->head_sentinel.next, __field); \
>  	(__node)->__field.next != NULL; 				\
>  	(__node) = exec_node_data(__type, (__node)->__field.next, __field))
>  
>  #define foreach_list_typed_reverse(__type, __node, __field, __list)        \
>     for (__type * __node =                                                \
> -           exec_node_data(__type, (__list)->tail_pred, __field);        \
> +           exec_node_data(__type, (__list)->tail_sentinel.prev, __field);  \
>          (__node)->__field.prev != NULL;                                 \
>          (__node) = exec_node_data(__type, (__node)->__field.prev, __field))
>  
>  #define foreach_list_typed_safe(__type, __node, __field, __list)           \
>     for (__type * __node =                                                  \
> -           exec_node_data(__type, (__list)->head, __field),                \
> +           exec_node_data(__type, (__list)->head_sentinel.next, __field),  \
>                 * __next =                                                  \
>             exec_node_data(__type, (__node)->__field.next, __field);        \
>          (__node)->__field.next != NULL;                                    \
> @@ -690,7 +668,7 @@ inline void exec_node::insert_before(exec_list *before)
>  
>  #define foreach_list_typed_reverse_safe(__type, __node, __field, __list)   \
>     for (__type * __node =                                                  \
> -           exec_node_data(__type, (__list)->tail_pred, __field),           \
> +           exec_node_data(__type, (__list)->tail_sentinel.prev, __field),  \
>                 * __prev =                                                  \
>             exec_node_data(__type, (__node)->__field.prev, __field);        \
>          (__node)->__field.prev != NULL;                                    \
> diff --git a/src/compiler/glsl/lower_distance.cpp b/src/compiler/glsl/lower_distance.cpp
> index c2158f2..cabdad6 100644
> --- a/src/compiler/glsl/lower_distance.cpp
> +++ b/src/compiler/glsl/lower_distance.cpp
> @@ -529,8 +529,8 @@ lower_distance_visitor::visit_leave(ir_call *ir)
>  {
>     void *ctx = ralloc_parent(ir);
>  
> -   const exec_node *formal_param_node = ir->callee->parameters.head;
> -   const exec_node *actual_param_node = ir->actual_parameters.head;
> +   const exec_node *formal_param_node = ir->callee->parameters.head_sentinel.next;
> +   const exec_node *actual_param_node = ir->actual_parameters.head_sentinel.next;
>     while (!actual_param_node->is_tail_sentinel()) {
>        ir_variable *formal_param = (ir_variable *) formal_param_node;
>        ir_rvalue *actual_param = (ir_rvalue *) actual_param_node;
> diff --git a/src/compiler/glsl/lower_jumps.cpp b/src/compiler/glsl/lower_jumps.cpp
> index 3cfa2e0..ed1b6d1 100644
> --- a/src/compiler/glsl/lower_jumps.cpp
> +++ b/src/compiler/glsl/lower_jumps.cpp
> @@ -777,7 +777,7 @@ lower_continue:
>                  * analysis.
>                  */
>                 exec_list list;
> -               list.head = next;
> +               list.head_sentinel.next = next;
>                 block_records[move_into] = visit_block(&list);
>  
>                 /*
> diff --git a/src/compiler/glsl/lower_packed_varyings.cpp b/src/compiler/glsl/lower_packed_varyings.cpp
> index 130b8f6..aa9b2b5 100644
> --- a/src/compiler/glsl/lower_packed_varyings.cpp
> +++ b/src/compiler/glsl/lower_packed_varyings.cpp
> @@ -791,7 +791,7 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used,
>           lower_packed_varyings_gs_splicer splicer(mem_ctx, &new_instructions);
>  
>           /* Add all the variables in first. */
> -         main_func_sig->body.head->insert_before(&new_variables);
> +         main_func_sig->body.head_sentinel.next->insert_before(&new_variables);
>  
>           /* Now update all the EmitVertex instances */
>           splicer.run(instructions);
> @@ -802,7 +802,7 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used,
>  
>           lower_packed_varyings_return_splicer splicer(mem_ctx, &new_instructions);
>  
> -         main_func_sig->body.head->insert_before(&new_variables);
> +         main_func_sig->body.head_sentinel.next->insert_before(&new_variables);
>  
>           splicer.run(instructions);
>  
> @@ -815,7 +815,7 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used,
>        }
>     } else {
>        /* Shader inputs need to be lowered at the beginning of main() */
> -      main_func_sig->body.head->insert_before(&new_instructions);
> -      main_func_sig->body.head->insert_before(&new_variables);
> +      main_func_sig->body.head_sentinel.next->insert_before(&new_instructions);
> +      main_func_sig->body.head_sentinel.next->insert_before(&new_variables);
>     }
>  }
> diff --git a/src/compiler/glsl/lower_tess_level.cpp b/src/compiler/glsl/lower_tess_level.cpp
> index bed2553..1f46999 100644
> --- a/src/compiler/glsl/lower_tess_level.cpp
> +++ b/src/compiler/glsl/lower_tess_level.cpp
> @@ -382,8 +382,8 @@ lower_tess_level_visitor::visit_leave(ir_call *ir)
>  {
>     void *ctx = ralloc_parent(ir);
>  
> -   const exec_node *formal_param_node = ir->callee->parameters.head;
> -   const exec_node *actual_param_node = ir->actual_parameters.head;
> +   const exec_node *formal_param_node = ir->callee->parameters.head_sentinel.next;
> +   const exec_node *actual_param_node = ir->actual_parameters.head_sentinel.next;
>     while (!actual_param_node->is_tail_sentinel()) {
>        ir_variable *formal_param = (ir_variable *) formal_param_node;
>        ir_rvalue *actual_param = (ir_rvalue *) actual_param_node;
> diff --git a/src/compiler/glsl/opt_conditional_discard.cpp b/src/compiler/glsl/opt_conditional_discard.cpp
> index 1ca8803..97eb4ba 100644
> --- a/src/compiler/glsl/opt_conditional_discard.cpp
> +++ b/src/compiler/glsl/opt_conditional_discard.cpp
> @@ -65,13 +65,13 @@ opt_conditional_discard_visitor::visit_leave(ir_if *ir)
>  {
>     /* Look for "if (...) discard" with no else clause or extra statements. */
>     if (ir->then_instructions.is_empty() ||
> -       !ir->then_instructions.head->next->is_tail_sentinel() ||
> -       !((ir_instruction *) ir->then_instructions.head)->as_discard() ||
> +       !ir->then_instructions.head_sentinel.next->next->is_tail_sentinel() ||
> +       !((ir_instruction *) ir->then_instructions.head_sentinel.next)->as_discard() ||
>         !ir->else_instructions.is_empty())
>        return visit_continue;
>  
>     /* Move the condition and replace the ir_if with the ir_discard. */
> -   ir_discard *discard = (ir_discard *) ir->then_instructions.head;
> +   ir_discard *discard = (ir_discard *) ir->then_instructions.head_sentinel.next;
>     discard->condition = ir->condition;
>     ir->replace_with(discard);
>  
> diff --git a/src/compiler/glsl/opt_dead_builtin_varyings.cpp b/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> index 2e40b78..5281a4f 100644
> --- a/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> +++ b/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> @@ -378,7 +378,7 @@ public:
>                 new_var[i]->data.explicit_index = 0;
>              }
>  
> -            ir->head->insert_before(new_var[i]);
> +            ir->head_sentinel.next->insert_before(new_var[i]);
>           }
>        }
>     }
> diff --git a/src/compiler/glsl/opt_dead_code.cpp b/src/compiler/glsl/opt_dead_code.cpp
> index 3560119..95cb7e0 100644
> --- a/src/compiler/glsl/opt_dead_code.cpp
> +++ b/src/compiler/glsl/opt_dead_code.cpp
> @@ -101,7 +101,7 @@ do_dead_code(exec_list *instructions, bool uniform_locations_assigned)
>              while (!entry->assign_list.is_empty()) {
>                 struct assignment_entry *assignment_entry =
>                    exec_node_data(struct assignment_entry,
> -                                 entry->assign_list.head, link);
> +                                 entry->assign_list.head_sentinel.next, link);
>  
>  	       assignment_entry->assign->remove();
>  
> diff --git a/src/compiler/glsl/opt_flatten_nested_if_blocks.cpp b/src/compiler/glsl/opt_flatten_nested_if_blocks.cpp
> index c702102..7257fe5 100644
> --- a/src/compiler/glsl/opt_flatten_nested_if_blocks.cpp
> +++ b/src/compiler/glsl/opt_flatten_nested_if_blocks.cpp
> @@ -90,7 +90,7 @@ nested_if_flattener::visit_leave(ir_if *ir)
>     if (ir->then_instructions.is_empty() || !ir->else_instructions.is_empty())
>        return visit_continue;
>  
> -   ir_if *inner = ((ir_instruction *) ir->then_instructions.head)->as_if();
> +   ir_if *inner = ((ir_instruction *) ir->then_instructions.head_sentinel.next)->as_if();
>     if (!inner || !inner->next->is_tail_sentinel() ||
>         !inner->else_instructions.is_empty())
>        return visit_continue;
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 1725ee3..46bbf03 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -1553,13 +1553,13 @@ typedef struct {
>  ATTRIBUTE_RETURNS_NONNULL static inline nir_block *
>  nir_start_block(nir_function_impl *impl)
>  {
> -   return (nir_block *) impl->body.head;
> +   return (nir_block *) impl->body.head_sentinel.next;
>  }
>  
>  ATTRIBUTE_RETURNS_NONNULL static inline nir_block *
>  nir_impl_last_block(nir_function_impl *impl)
>  {
> -   return (nir_block *) impl->body.tail_pred;
> +   return (nir_block *) impl->body.tail_sentinel.prev;
>  }
>  
>  static inline nir_cf_node *
> diff --git a/src/compiler/nir/nir_opt_gcm.c b/src/compiler/nir/nir_opt_gcm.c
> index ff079b9..84e32ef 100644
> --- a/src/compiler/nir/nir_opt_gcm.c
> +++ b/src/compiler/nir/nir_opt_gcm.c
> @@ -478,7 +478,7 @@ opt_gcm_impl(nir_function_impl *impl)
>  
>     while (!exec_list_is_empty(&state.instrs)) {
>        nir_instr *instr = exec_node_data(nir_instr,
> -                                        state.instrs.tail_pred, node);
> +                                        state.instrs.tail_sentinel.prev, node);
>        gcm_place_instr(instr, &state);
>     }
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h b/src/mesa/drivers/dri/i965/brw_cfg.h
> index 1c90eab..b8af40f 100644
> --- a/src/mesa/drivers/dri/i965/brw_cfg.h
> +++ b/src/mesa/drivers/dri/i965/brw_cfg.h
> @@ -333,7 +333,7 @@ struct cfg_t {
>     foreach_in_list(__type, __inst, &(__block)->instructions)
>  
>  #define foreach_inst_in_block_safe(__type, __inst, __block)    \
> -   for (__type *__inst = (__type *)__block->instructions.head, \
> +   for (__type *__inst = (__type *)__block->instructions.head_sentinel.next, \
>                 *__next = (__type *)__inst->next;               \
>          __next != NULL;                                        \
>          __inst = __next,                                       \
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_builder.h b/src/mesa/drivers/dri/i965/brw_fs_builder.h
> index f22903e..173c3ce 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_builder.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs_builder.h
> @@ -101,7 +101,7 @@ namespace brw {
>        fs_builder
>        at_end() const
>        {
> -         return at(NULL, (exec_node *)&shader->instructions.tail);
> +         return at(NULL, (exec_node *)&shader->instructions.tail_sentinel);
>        }
>  
>        /**
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_builder.h b/src/mesa/drivers/dri/i965/brw_vec4_builder.h
> index 3a8617e..96612c7 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_builder.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_builder.h
> @@ -95,7 +95,7 @@ namespace brw {
>        vec4_builder
>        at_end() const
>        {
> -         return at(NULL, (exec_node *)&shader->instructions.tail);
> +         return at(NULL, (exec_node *)&shader->instructions.tail_sentinel);
>        }
>  
>        /**
> 



More information about the mesa-dev mailing list