tree, c++: optimize walk_tree_1 and cp_walk_subtrees

  These functions currently repeatedly dereference tp during the subtree
walk, dereferences which the compiler can't CSE because it can't
guarantee that the subtree walking doesn't modify *tp.

But we already implicitly require that TREE_CODE (*tp) remains the same
throughout the subtree walks, so it doesn't seem to be a huge leap to
strengthen that to requiring *tp remains the same.

So this patch manually CSEs the dereferences of *tp.  This means that
the callback function can no longer replace *tp with another tree (of
the same TREE_CODE) when walking one of its subtrees, but that doesn't
sound like a useful feature anyway.  This speeds up the C++ frontend by
about ~1.5% according to my experiments.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk stage3 or perhaps for stage1?

gcc/cp/ChangeLog:

	* tree.cc (cp_walk_subtrees): Avoid repeatedly dereferencing tp.

gcc/ChangeLog:

	* tree.cc (walk_tree_1): Avoid repeatedly dereferencing tp
	and type_p.
---
 gcc/cp/tree.cc | 113 +++++++++++++++++++++++++------------------------
 gcc/tree.cc    | 103 ++++++++++++++++++++++----------------------
 2 files changed, 108 insertions(+), 108 deletions(-)
  

On Mon, 5 Dec 2022 at 09:51, Patrick Palka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> These functions currently repeatedly dereference tp during the subtree
> walk, dereferences which the compiler can't CSE because it can't
> guarantee that the subtree walking doesn't modify *tp.
>
> But we already implicitly require that TREE_CODE (*tp) remains the same
> throughout the subtree walks, so it doesn't seem to be a huge leap to
> strengthen that to requiring *tp remains the same.
Hi Patrick,
Just wondering in that case, if marking tp with const_tree *, instead
of tree *, would perhaps help the compiler
for CSEing some of the dereferences to *tp ?

Thanks,
Prathamesh
>
> So this patch manually CSEs the dereferences of *tp.  This means that
> the callback function can no longer replace *tp with another tree (of
> the same TREE_CODE) when walking one of its subtrees, but that doesn't
> sound like a useful feature anyway.  This speeds up the C++ frontend by
> about ~1.5% according to my experiments.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk stage3 or perhaps for stage1?
>
> gcc/cp/ChangeLog:
>
>         * tree.cc (cp_walk_subtrees): Avoid repeatedly dereferencing tp.
>
> gcc/ChangeLog:
>
>         * tree.cc (walk_tree_1): Avoid repeatedly dereferencing tp
>         and type_p.
> ---
>  gcc/cp/tree.cc | 113 +++++++++++++++++++++++++------------------------
>  gcc/tree.cc    | 103 ++++++++++++++++++++++----------------------
>  2 files changed, 108 insertions(+), 108 deletions(-)
>
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 4066b014f6e..ceb0c75f559 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5441,7 +5441,8 @@ tree
>  cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>                   void *data, hash_set<tree> *pset)
>  {
> -  enum tree_code code = TREE_CODE (*tp);
> +  tree t = *tp;
> +  enum tree_code code = TREE_CODE (t);
>    tree result;
>
>  #define WALK_SUBTREE(NODE)                             \
> @@ -5452,7 +5453,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>      }                                                  \
>    while (0)
>
> -  if (TYPE_P (*tp))
> +  if (TYPE_P (t))
>      {
>        /* If *WALK_SUBTREES_P is 1, we're interested in the syntactic form of
>          the argument, so don't look through typedefs, but do walk into
> @@ -5464,15 +5465,15 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>
>          See find_abi_tags_r for an example of setting *WALK_SUBTREES_P to 2
>          when that's the behavior the walk_tree_fn wants.  */
> -      if (*walk_subtrees_p == 1 && typedef_variant_p (*tp))
> +      if (*walk_subtrees_p == 1 && typedef_variant_p (t))
>         {
> -         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (*tp))
> +         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (t))
>             WALK_SUBTREE (TI_ARGS (ti));
>           *walk_subtrees_p = 0;
>           return NULL_TREE;
>         }
>
> -      if (tree ti = TYPE_TEMPLATE_INFO (*tp))
> +      if (tree ti = TYPE_TEMPLATE_INFO (t))
>         WALK_SUBTREE (TI_ARGS (ti));
>      }
>
> @@ -5482,8 +5483,8 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>    switch (code)
>      {
>      case TEMPLATE_TYPE_PARM:
> -      if (template_placeholder_p (*tp))
> -       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (*tp));
> +      if (template_placeholder_p (t))
> +       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (t));
>        /* Fall through.  */
>      case DEFERRED_PARSE:
>      case TEMPLATE_TEMPLATE_PARM:
> @@ -5497,63 +5498,63 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>        break;
>
>      case TYPENAME_TYPE:
> -      WALK_SUBTREE (TYPE_CONTEXT (*tp));
> -      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (*tp));
> +      WALK_SUBTREE (TYPE_CONTEXT (t));
> +      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case BASELINK:
> -      if (BASELINK_QUALIFIED_P (*tp))
> -       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (*tp)));
> -      WALK_SUBTREE (BASELINK_FUNCTIONS (*tp));
> +      if (BASELINK_QUALIFIED_P (t))
> +       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (t)));
> +      WALK_SUBTREE (BASELINK_FUNCTIONS (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case PTRMEM_CST:
> -      WALK_SUBTREE (TREE_TYPE (*tp));
> +      WALK_SUBTREE (TREE_TYPE (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case TREE_LIST:
> -      WALK_SUBTREE (TREE_PURPOSE (*tp));
> +      WALK_SUBTREE (TREE_PURPOSE (t));
>        break;
>
>      case OVERLOAD:
> -      WALK_SUBTREE (OVL_FUNCTION (*tp));
> -      WALK_SUBTREE (OVL_CHAIN (*tp));
> +      WALK_SUBTREE (OVL_FUNCTION (t));
> +      WALK_SUBTREE (OVL_CHAIN (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case USING_DECL:
> -      WALK_SUBTREE (DECL_NAME (*tp));
> -      WALK_SUBTREE (USING_DECL_SCOPE (*tp));
> -      WALK_SUBTREE (USING_DECL_DECLS (*tp));
> +      WALK_SUBTREE (DECL_NAME (t));
> +      WALK_SUBTREE (USING_DECL_SCOPE (t));
> +      WALK_SUBTREE (USING_DECL_DECLS (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case RECORD_TYPE:
> -      if (TYPE_PTRMEMFUNC_P (*tp))
> -       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (*tp));
> +      if (TYPE_PTRMEMFUNC_P (t))
> +       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (t));
>        break;
>
>      case TYPE_ARGUMENT_PACK:
>      case NONTYPE_ARGUMENT_PACK:
>        {
> -        tree args = ARGUMENT_PACK_ARGS (*tp);
> +       tree args = ARGUMENT_PACK_ARGS (t);
>         for (tree arg : tree_vec_range (args))
>           WALK_SUBTREE (arg);
>        }
>        break;
>
>      case TYPE_PACK_EXPANSION:
> -      WALK_SUBTREE (TREE_TYPE (*tp));
> -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
> +      WALK_SUBTREE (TREE_TYPE (t));
> +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case EXPR_PACK_EXPANSION:
> -      WALK_SUBTREE (TREE_OPERAND (*tp, 0));
> -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
> +      WALK_SUBTREE (TREE_OPERAND (t, 0));
> +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
>        *walk_subtrees_p = 0;
>        break;
>
> @@ -5564,31 +5565,31 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>      case DYNAMIC_CAST_EXPR:
>      case IMPLICIT_CONV_EXPR:
>      case BIT_CAST_EXPR:
> -      if (TREE_TYPE (*tp))
> -       WALK_SUBTREE (TREE_TYPE (*tp));
> +      if (TREE_TYPE (t))
> +       WALK_SUBTREE (TREE_TYPE (t));
>        break;
>
>      case CONSTRUCTOR:
> -      if (COMPOUND_LITERAL_P (*tp))
> -       WALK_SUBTREE (TREE_TYPE (*tp));
> +      if (COMPOUND_LITERAL_P (t))
> +       WALK_SUBTREE (TREE_TYPE (t));
>        break;
>
>      case TRAIT_EXPR:
> -      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (*tp));
> -      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (*tp));
> +      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (t));
> +      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case TRAIT_TYPE:
> -      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (*tp));
> -      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (*tp));
> +      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (t));
> +      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (t));
>        *walk_subtrees_p = 0;
>        break;
>
>      case DECLTYPE_TYPE:
>        ++cp_unevaluated_operand;
>        /* We can't use WALK_SUBTREE here because of the goto.  */
> -      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (*tp), func, data, pset);
> +      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (t), func, data, pset);
>        --cp_unevaluated_operand;
>        *walk_subtrees_p = 0;
>        break;
> @@ -5597,7 +5598,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>      case SIZEOF_EXPR:
>      case NOEXCEPT_EXPR:
>        ++cp_unevaluated_operand;
> -      result = cp_walk_tree (&TREE_OPERAND (*tp, 0), func, data, pset);
> +      result = cp_walk_tree (&TREE_OPERAND (t, 0), func, data, pset);
>        --cp_unevaluated_operand;
>        *walk_subtrees_p = 0;
>        break;
> @@ -5605,12 +5606,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>      case REQUIRES_EXPR:
>        {
>         cp_unevaluated u;
> -       for (tree parm = REQUIRES_EXPR_PARMS (*tp); parm; parm = DECL_CHAIN (parm))
> +       for (tree parm = REQUIRES_EXPR_PARMS (t); parm; parm = DECL_CHAIN (parm))
>           /* Walk the types of each parameter, but not the parameter itself.
>              Doing so would cause false positives in the unexpanded parameter
>              pack checker since an introduced parameter pack looks bare.  */
>           WALK_SUBTREE (TREE_TYPE (parm));
> -       WALK_SUBTREE (REQUIRES_EXPR_REQS (*tp));
> +       WALK_SUBTREE (REQUIRES_EXPR_REQS (t));
>         *walk_subtrees_p = 0;
>         break;
>        }
> @@ -5621,12 +5622,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>          the containing BIND_EXPR.  Compiler temporaries are
>          handled here.  And also normal variables in templates,
>          since do_poplevel doesn't build a BIND_EXPR then.  */
> -      if (VAR_P (TREE_OPERAND (*tp, 0))
> +      if (VAR_P (TREE_OPERAND (t, 0))
>           && (processing_template_decl
> -             || (DECL_ARTIFICIAL (TREE_OPERAND (*tp, 0))
> -                 && !TREE_STATIC (TREE_OPERAND (*tp, 0)))))
> +             || (DECL_ARTIFICIAL (TREE_OPERAND (t, 0))
> +                 && !TREE_STATIC (TREE_OPERAND (t, 0)))))
>         {
> -         tree decl = TREE_OPERAND (*tp, 0);
> +         tree decl = TREE_OPERAND (t, 0);
>           WALK_SUBTREE (DECL_INITIAL (decl));
>           WALK_SUBTREE (DECL_SIZE (decl));
>           WALK_SUBTREE (DECL_SIZE_UNIT (decl));
> @@ -5636,45 +5637,45 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>      case LAMBDA_EXPR:
>        /* Don't walk into the body of the lambda, but the capture initializers
>          are part of the enclosing context.  */
> -      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (*tp); cap;
> +      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (t); cap;
>            cap = TREE_CHAIN (cap))
>         WALK_SUBTREE (TREE_VALUE (cap));
>        break;
>
>      case CO_YIELD_EXPR:
> -      if (TREE_OPERAND (*tp, 1))
> +      if (TREE_OPERAND (t, 1))
>         /* Operand 1 is the tree for the relevant co_await which has any
>            interesting sub-trees.  */
> -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> +       WALK_SUBTREE (TREE_OPERAND (t, 1));
>        break;
>
>      case CO_AWAIT_EXPR:
> -      if (TREE_OPERAND (*tp, 1))
> +      if (TREE_OPERAND (t, 1))
>         /* Operand 1 is frame variable.  */
> -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> -      if (TREE_OPERAND (*tp, 2))
> +       WALK_SUBTREE (TREE_OPERAND (t, 1));
> +      if (TREE_OPERAND (t, 2))
>         /* Operand 2 has the initialiser, and we need to walk any subtrees
>            there.  */
> -       WALK_SUBTREE (TREE_OPERAND (*tp, 2));
> +       WALK_SUBTREE (TREE_OPERAND (t, 2));
>        break;
>
>      case CO_RETURN_EXPR:
> -      if (TREE_OPERAND (*tp, 0))
> +      if (TREE_OPERAND (t, 0))
>         {
> -         if (VOID_TYPE_P (TREE_OPERAND (*tp, 0)))
> +         if (VOID_TYPE_P (TREE_OPERAND (t, 0)))
>             /* For void expressions, operand 1 is a trivial call, and any
>                interesting subtrees will be part of operand 0.  */
> -           WALK_SUBTREE (TREE_OPERAND (*tp, 0));
> -         else if (TREE_OPERAND (*tp, 1))
> +           WALK_SUBTREE (TREE_OPERAND (t, 0));
> +         else if (TREE_OPERAND (t, 1))
>             /* Interesting sub-trees will be in the return_value () call
>                arguments.  */
> -           WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> +           WALK_SUBTREE (TREE_OPERAND (t, 1));
>         }
>        break;
>
>      case STATIC_ASSERT:
> -      WALK_SUBTREE (STATIC_ASSERT_CONDITION (*tp));
> -      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (*tp));
> +      WALK_SUBTREE (STATIC_ASSERT_CONDITION (t));
> +      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (t));
>        break;
>
>      default:
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 5eb6a49da31..dfada652570 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -11218,10 +11218,6 @@ tree
>  walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>              hash_set<tree> *pset, walk_tree_lh lh)
>  {
> -  enum tree_code code;
> -  int walk_subtrees;
> -  tree result;
> -
>  #define WALK_SUBTREE_TAIL(NODE)                                \
>    do                                                   \
>      {                                                  \
> @@ -11241,14 +11237,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>      return NULL_TREE;
>
>    /* Call the function.  */
> -  walk_subtrees = 1;
> -  result = (*func) (tp, &walk_subtrees, data);
> +  int walk_subtrees = 1;
> +  tree result = (*func) (tp, &walk_subtrees, data);
>
>    /* If we found something, return it.  */
>    if (result)
>      return result;
>
> -  code = TREE_CODE (*tp);
> +  tree t = *tp;
> +  tree_code code = TREE_CODE (t);
>
>    /* Even if we didn't, FUNC may have decided that there was nothing
>       interesting below this point in the tree.  */
> @@ -11256,9 +11253,9 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>      {
>        /* But we still need to check our siblings.  */
>        if (code == TREE_LIST)
> -       WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
> +       WALK_SUBTREE_TAIL (TREE_CHAIN (t));
>        else if (code == OMP_CLAUSE)
> -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
> +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
>        else
>         return NULL_TREE;
>      }
> @@ -11288,58 +11285,58 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>        break;
>
>      case TREE_LIST:
> -      WALK_SUBTREE (TREE_VALUE (*tp));
> -      WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
> +      WALK_SUBTREE (TREE_VALUE (t));
> +      WALK_SUBTREE_TAIL (TREE_CHAIN (t));
>
>      case TREE_VEC:
>        {
> -       int len = TREE_VEC_LENGTH (*tp);
> +       int len = TREE_VEC_LENGTH (t);
>
>         if (len == 0)
>           break;
>
>         /* Walk all elements but the first.  */
>         while (--len)
> -         WALK_SUBTREE (TREE_VEC_ELT (*tp, len));
> +         WALK_SUBTREE (TREE_VEC_ELT (t, len));
>
>         /* Now walk the first one as a tail call.  */
> -       WALK_SUBTREE_TAIL (TREE_VEC_ELT (*tp, 0));
> +       WALK_SUBTREE_TAIL (TREE_VEC_ELT (t, 0));
>        }
>
>      case VECTOR_CST:
>        {
> -       unsigned len = vector_cst_encoded_nelts (*tp);
> +       unsigned len = vector_cst_encoded_nelts (t);
>         if (len == 0)
>           break;
>         /* Walk all elements but the first.  */
>         while (--len)
> -         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (*tp, len));
> +         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (t, len));
>         /* Now walk the first one as a tail call.  */
> -       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (*tp, 0));
> +       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (t, 0));
>        }
>
>      case COMPLEX_CST:
> -      WALK_SUBTREE (TREE_REALPART (*tp));
> -      WALK_SUBTREE_TAIL (TREE_IMAGPART (*tp));
> +      WALK_SUBTREE (TREE_REALPART (t));
> +      WALK_SUBTREE_TAIL (TREE_IMAGPART (t));
>
>      case CONSTRUCTOR:
>        {
>         unsigned HOST_WIDE_INT idx;
>         constructor_elt *ce;
>
> -       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (*tp), idx, &ce);
> +       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (t), idx, &ce);
>              idx++)
>           WALK_SUBTREE (ce->value);
>        }
>        break;
>
>      case SAVE_EXPR:
> -      WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, 0));
> +      WALK_SUBTREE_TAIL (TREE_OPERAND (t, 0));
>
>      case BIND_EXPR:
>        {
>         tree decl;
> -       for (decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
> +       for (decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
>           {
>             /* Walk the DECL_INITIAL and DECL_SIZE.  We don't want to walk
>                into declarations that are just mentioned, rather than
> @@ -11350,23 +11347,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>             WALK_SUBTREE (DECL_SIZE (decl));
>             WALK_SUBTREE (DECL_SIZE_UNIT (decl));
>           }
> -       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (*tp));
> +       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (t));
>        }
>
>      case STATEMENT_LIST:
>        {
>         tree_stmt_iterator i;
> -       for (i = tsi_start (*tp); !tsi_end_p (i); tsi_next (&i))
> +       for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
>           WALK_SUBTREE (*tsi_stmt_ptr (i));
>        }
>        break;
>
>      case OMP_CLAUSE:
>        {
> -       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (*tp)];
> +       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (t)];
>         for (int i = 0; i < len; i++)
> -         WALK_SUBTREE (OMP_CLAUSE_OPERAND (*tp, i));
> -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
> +         WALK_SUBTREE (OMP_CLAUSE_OPERAND (t, i));
> +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
>        }
>
>      case TARGET_EXPR:
> @@ -11375,10 +11372,10 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>
>         /* TARGET_EXPRs are peculiar: operands 1 and 3 can be the same.
>            But, we only want to walk once.  */
> -       len = (TREE_OPERAND (*tp, 3) == TREE_OPERAND (*tp, 1)) ? 2 : 3;
> +       len = (TREE_OPERAND (t, 3) == TREE_OPERAND (t, 1)) ? 2 : 3;
>         for (i = 0; i < len; ++i)
> -         WALK_SUBTREE (TREE_OPERAND (*tp, i));
> -       WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len));
> +         WALK_SUBTREE (TREE_OPERAND (t, i));
> +       WALK_SUBTREE_TAIL (TREE_OPERAND (t, len));
>        }
>
>      case DECL_EXPR:
> @@ -11393,15 +11390,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>          variable-sized types.
>
>          Note that DECLs get walked as part of processing the BIND_EXPR.  */
> -      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL)
> +      if (TREE_CODE (DECL_EXPR_DECL (t)) == TYPE_DECL)
>         {
>           /* Call the function for the decl so e.g. copy_tree_body_r can
>              replace it with the remapped one.  */
> -         result = (*func) (&DECL_EXPR_DECL (*tp), &walk_subtrees, data);
> +         result = (*func) (&DECL_EXPR_DECL (t), &walk_subtrees, data);
>           if (result || !walk_subtrees)
>             return result;
>
> -         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (*tp));
> +         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (t));
>           if (TREE_CODE (*type_p) == ERROR_MARK)
>             return NULL_TREE;
>
> @@ -11412,21 +11409,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>           if (result || !walk_subtrees)
>             return result;
>
> +         tree type = *type_p;
> +
>           /* But do not walk a pointed-to type since it may itself need to
>              be walked in the declaration case if it isn't anonymous.  */
> -         if (!POINTER_TYPE_P (*type_p))
> +         if (!POINTER_TYPE_P (type))
>             {
> -             result = walk_type_fields (*type_p, func, data, pset, lh);
> +             result = walk_type_fields (type, func, data, pset, lh);
>               if (result)
>                 return result;
>             }
>
>           /* If this is a record type, also walk the fields.  */
> -         if (RECORD_OR_UNION_TYPE_P (*type_p))
> +         if (RECORD_OR_UNION_TYPE_P (type))
>             {
>               tree field;
>
> -             for (field = TYPE_FIELDS (*type_p); field;
> +             for (field = TYPE_FIELDS (type); field;
>                    field = DECL_CHAIN (field))
>                 {
>                   /* We'd like to look at the type of the field, but we can
> @@ -11439,24 +11438,24 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>                   WALK_SUBTREE (DECL_FIELD_OFFSET (field));
>                   WALK_SUBTREE (DECL_SIZE (field));
>                   WALK_SUBTREE (DECL_SIZE_UNIT (field));
> -                 if (TREE_CODE (*type_p) == QUAL_UNION_TYPE)
> +                 if (TREE_CODE (type) == QUAL_UNION_TYPE)
>                     WALK_SUBTREE (DECL_QUALIFIER (field));
>                 }
>             }
>
>           /* Same for scalar types.  */
> -         else if (TREE_CODE (*type_p) == BOOLEAN_TYPE
> -                  || TREE_CODE (*type_p) == ENUMERAL_TYPE
> -                  || TREE_CODE (*type_p) == INTEGER_TYPE
> -                  || TREE_CODE (*type_p) == FIXED_POINT_TYPE
> -                  || TREE_CODE (*type_p) == REAL_TYPE)
> +         else if (TREE_CODE (type) == BOOLEAN_TYPE
> +                  || TREE_CODE (type) == ENUMERAL_TYPE
> +                  || TREE_CODE (type) == INTEGER_TYPE
> +                  || TREE_CODE (type) == FIXED_POINT_TYPE
> +                  || TREE_CODE (type) == REAL_TYPE)
>             {
> -             WALK_SUBTREE (TYPE_MIN_VALUE (*type_p));
> -             WALK_SUBTREE (TYPE_MAX_VALUE (*type_p));
> +             WALK_SUBTREE (TYPE_MIN_VALUE (type));
> +             WALK_SUBTREE (TYPE_MAX_VALUE (type));
>             }
>
> -         WALK_SUBTREE (TYPE_SIZE (*type_p));
> -         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (*type_p));
> +         WALK_SUBTREE (TYPE_SIZE (type));
> +         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (type));
>         }
>        /* FALLTHRU */
>
> @@ -11466,20 +11465,20 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>           int i, len;
>
>           /* Walk over all the sub-trees of this operand.  */
> -         len = TREE_OPERAND_LENGTH (*tp);
> +         len = TREE_OPERAND_LENGTH (t);
>
>           /* Go through the subtrees.  We need to do this in forward order so
>              that the scope of a FOR_EXPR is handled properly.  */
>           if (len)
>             {
>               for (i = 0; i < len - 1; ++i)
> -               WALK_SUBTREE (TREE_OPERAND (*tp, i));
> -             WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len - 1));
> +               WALK_SUBTREE (TREE_OPERAND (t, i));
> +             WALK_SUBTREE_TAIL (TREE_OPERAND (t, len - 1));
>             }
>         }
>        /* If this is a type, walk the needed fields in the type.  */
> -      else if (TYPE_P (*tp))
> -       return walk_type_fields (*tp, func, data, pset, lh);
> +      else if (TYPE_P (t))
> +       return walk_type_fields (t, func, data, pset, lh);
>        break;
>      }
>
> --
> 2.39.0.rc0.49.g083e01275b
>
  

On Mon, 5 Dec 2022, Prathamesh Kulkarni wrote:

> On Mon, 5 Dec 2022 at 09:51, Patrick Palka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > These functions currently repeatedly dereference tp during the subtree
> > walk, dereferences which the compiler can't CSE because it can't
> > guarantee that the subtree walking doesn't modify *tp.
> >
> > But we already implicitly require that TREE_CODE (*tp) remains the same
> > throughout the subtree walks, so it doesn't seem to be a huge leap to
> > strengthen that to requiring *tp remains the same.
> Hi Patrick,
> Just wondering in that case, if marking tp with const_tree *, instead
> of tree *, would perhaps help the compiler
> for CSEing some of the dereferences to *tp ?

IIUC I don't think using const_tree will help optimization-wise since the
main issue is CSEing across calls to the callback fn, and the callback
can still potentially modify *tp through some other, non-const pointer
to it since it's global memory.

> 
> Thanks,
> Prathamesh
> >
> > So this patch manually CSEs the dereferences of *tp.  This means that
> > the callback function can no longer replace *tp with another tree (of
> > the same TREE_CODE) when walking one of its subtrees, but that doesn't
> > sound like a useful feature anyway.  This speeds up the C++ frontend by
> > about ~1.5% according to my experiments.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk stage3 or perhaps for stage1?
> >
> > gcc/cp/ChangeLog:
> >
> >         * tree.cc (cp_walk_subtrees): Avoid repeatedly dereferencing tp.
> >
> > gcc/ChangeLog:
> >
> >         * tree.cc (walk_tree_1): Avoid repeatedly dereferencing tp
> >         and type_p.
> > ---
> >  gcc/cp/tree.cc | 113 +++++++++++++++++++++++++------------------------
> >  gcc/tree.cc    | 103 ++++++++++++++++++++++----------------------
> >  2 files changed, 108 insertions(+), 108 deletions(-)
> >
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index 4066b014f6e..ceb0c75f559 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -5441,7 +5441,8 @@ tree
> >  cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >                   void *data, hash_set<tree> *pset)
> >  {
> > -  enum tree_code code = TREE_CODE (*tp);
> > +  tree t = *tp;
> > +  enum tree_code code = TREE_CODE (t);
> >    tree result;
> >
> >  #define WALK_SUBTREE(NODE)                             \
> > @@ -5452,7 +5453,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >      }                                                  \
> >    while (0)
> >
> > -  if (TYPE_P (*tp))
> > +  if (TYPE_P (t))
> >      {
> >        /* If *WALK_SUBTREES_P is 1, we're interested in the syntactic form of
> >          the argument, so don't look through typedefs, but do walk into
> > @@ -5464,15 +5465,15 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >
> >          See find_abi_tags_r for an example of setting *WALK_SUBTREES_P to 2
> >          when that's the behavior the walk_tree_fn wants.  */
> > -      if (*walk_subtrees_p == 1 && typedef_variant_p (*tp))
> > +      if (*walk_subtrees_p == 1 && typedef_variant_p (t))
> >         {
> > -         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (*tp))
> > +         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (t))
> >             WALK_SUBTREE (TI_ARGS (ti));
> >           *walk_subtrees_p = 0;
> >           return NULL_TREE;
> >         }
> >
> > -      if (tree ti = TYPE_TEMPLATE_INFO (*tp))
> > +      if (tree ti = TYPE_TEMPLATE_INFO (t))
> >         WALK_SUBTREE (TI_ARGS (ti));
> >      }
> >
> > @@ -5482,8 +5483,8 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >    switch (code)
> >      {
> >      case TEMPLATE_TYPE_PARM:
> > -      if (template_placeholder_p (*tp))
> > -       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (*tp));
> > +      if (template_placeholder_p (t))
> > +       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (t));
> >        /* Fall through.  */
> >      case DEFERRED_PARSE:
> >      case TEMPLATE_TEMPLATE_PARM:
> > @@ -5497,63 +5498,63 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >        break;
> >
> >      case TYPENAME_TYPE:
> > -      WALK_SUBTREE (TYPE_CONTEXT (*tp));
> > -      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (*tp));
> > +      WALK_SUBTREE (TYPE_CONTEXT (t));
> > +      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case BASELINK:
> > -      if (BASELINK_QUALIFIED_P (*tp))
> > -       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (*tp)));
> > -      WALK_SUBTREE (BASELINK_FUNCTIONS (*tp));
> > +      if (BASELINK_QUALIFIED_P (t))
> > +       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (t)));
> > +      WALK_SUBTREE (BASELINK_FUNCTIONS (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case PTRMEM_CST:
> > -      WALK_SUBTREE (TREE_TYPE (*tp));
> > +      WALK_SUBTREE (TREE_TYPE (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case TREE_LIST:
> > -      WALK_SUBTREE (TREE_PURPOSE (*tp));
> > +      WALK_SUBTREE (TREE_PURPOSE (t));
> >        break;
> >
> >      case OVERLOAD:
> > -      WALK_SUBTREE (OVL_FUNCTION (*tp));
> > -      WALK_SUBTREE (OVL_CHAIN (*tp));
> > +      WALK_SUBTREE (OVL_FUNCTION (t));
> > +      WALK_SUBTREE (OVL_CHAIN (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case USING_DECL:
> > -      WALK_SUBTREE (DECL_NAME (*tp));
> > -      WALK_SUBTREE (USING_DECL_SCOPE (*tp));
> > -      WALK_SUBTREE (USING_DECL_DECLS (*tp));
> > +      WALK_SUBTREE (DECL_NAME (t));
> > +      WALK_SUBTREE (USING_DECL_SCOPE (t));
> > +      WALK_SUBTREE (USING_DECL_DECLS (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case RECORD_TYPE:
> > -      if (TYPE_PTRMEMFUNC_P (*tp))
> > -       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (*tp));
> > +      if (TYPE_PTRMEMFUNC_P (t))
> > +       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (t));
> >        break;
> >
> >      case TYPE_ARGUMENT_PACK:
> >      case NONTYPE_ARGUMENT_PACK:
> >        {
> > -        tree args = ARGUMENT_PACK_ARGS (*tp);
> > +       tree args = ARGUMENT_PACK_ARGS (t);
> >         for (tree arg : tree_vec_range (args))
> >           WALK_SUBTREE (arg);
> >        }
> >        break;
> >
> >      case TYPE_PACK_EXPANSION:
> > -      WALK_SUBTREE (TREE_TYPE (*tp));
> > -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
> > +      WALK_SUBTREE (TREE_TYPE (t));
> > +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case EXPR_PACK_EXPANSION:
> > -      WALK_SUBTREE (TREE_OPERAND (*tp, 0));
> > -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
> > +      WALK_SUBTREE (TREE_OPERAND (t, 0));
> > +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> > @@ -5564,31 +5565,31 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >      case DYNAMIC_CAST_EXPR:
> >      case IMPLICIT_CONV_EXPR:
> >      case BIT_CAST_EXPR:
> > -      if (TREE_TYPE (*tp))
> > -       WALK_SUBTREE (TREE_TYPE (*tp));
> > +      if (TREE_TYPE (t))
> > +       WALK_SUBTREE (TREE_TYPE (t));
> >        break;
> >
> >      case CONSTRUCTOR:
> > -      if (COMPOUND_LITERAL_P (*tp))
> > -       WALK_SUBTREE (TREE_TYPE (*tp));
> > +      if (COMPOUND_LITERAL_P (t))
> > +       WALK_SUBTREE (TREE_TYPE (t));
> >        break;
> >
> >      case TRAIT_EXPR:
> > -      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (*tp));
> > -      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (*tp));
> > +      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (t));
> > +      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case TRAIT_TYPE:
> > -      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (*tp));
> > -      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (*tp));
> > +      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (t));
> > +      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (t));
> >        *walk_subtrees_p = 0;
> >        break;
> >
> >      case DECLTYPE_TYPE:
> >        ++cp_unevaluated_operand;
> >        /* We can't use WALK_SUBTREE here because of the goto.  */
> > -      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (*tp), func, data, pset);
> > +      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (t), func, data, pset);
> >        --cp_unevaluated_operand;
> >        *walk_subtrees_p = 0;
> >        break;
> > @@ -5597,7 +5598,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >      case SIZEOF_EXPR:
> >      case NOEXCEPT_EXPR:
> >        ++cp_unevaluated_operand;
> > -      result = cp_walk_tree (&TREE_OPERAND (*tp, 0), func, data, pset);
> > +      result = cp_walk_tree (&TREE_OPERAND (t, 0), func, data, pset);
> >        --cp_unevaluated_operand;
> >        *walk_subtrees_p = 0;
> >        break;
> > @@ -5605,12 +5606,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >      case REQUIRES_EXPR:
> >        {
> >         cp_unevaluated u;
> > -       for (tree parm = REQUIRES_EXPR_PARMS (*tp); parm; parm = DECL_CHAIN (parm))
> > +       for (tree parm = REQUIRES_EXPR_PARMS (t); parm; parm = DECL_CHAIN (parm))
> >           /* Walk the types of each parameter, but not the parameter itself.
> >              Doing so would cause false positives in the unexpanded parameter
> >              pack checker since an introduced parameter pack looks bare.  */
> >           WALK_SUBTREE (TREE_TYPE (parm));
> > -       WALK_SUBTREE (REQUIRES_EXPR_REQS (*tp));
> > +       WALK_SUBTREE (REQUIRES_EXPR_REQS (t));
> >         *walk_subtrees_p = 0;
> >         break;
> >        }
> > @@ -5621,12 +5622,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >          the containing BIND_EXPR.  Compiler temporaries are
> >          handled here.  And also normal variables in templates,
> >          since do_poplevel doesn't build a BIND_EXPR then.  */
> > -      if (VAR_P (TREE_OPERAND (*tp, 0))
> > +      if (VAR_P (TREE_OPERAND (t, 0))
> >           && (processing_template_decl
> > -             || (DECL_ARTIFICIAL (TREE_OPERAND (*tp, 0))
> > -                 && !TREE_STATIC (TREE_OPERAND (*tp, 0)))))
> > +             || (DECL_ARTIFICIAL (TREE_OPERAND (t, 0))
> > +                 && !TREE_STATIC (TREE_OPERAND (t, 0)))))
> >         {
> > -         tree decl = TREE_OPERAND (*tp, 0);
> > +         tree decl = TREE_OPERAND (t, 0);
> >           WALK_SUBTREE (DECL_INITIAL (decl));
> >           WALK_SUBTREE (DECL_SIZE (decl));
> >           WALK_SUBTREE (DECL_SIZE_UNIT (decl));
> > @@ -5636,45 +5637,45 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
> >      case LAMBDA_EXPR:
> >        /* Don't walk into the body of the lambda, but the capture initializers
> >          are part of the enclosing context.  */
> > -      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (*tp); cap;
> > +      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (t); cap;
> >            cap = TREE_CHAIN (cap))
> >         WALK_SUBTREE (TREE_VALUE (cap));
> >        break;
> >
> >      case CO_YIELD_EXPR:
> > -      if (TREE_OPERAND (*tp, 1))
> > +      if (TREE_OPERAND (t, 1))
> >         /* Operand 1 is the tree for the relevant co_await which has any
> >            interesting sub-trees.  */
> > -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> > +       WALK_SUBTREE (TREE_OPERAND (t, 1));
> >        break;
> >
> >      case CO_AWAIT_EXPR:
> > -      if (TREE_OPERAND (*tp, 1))
> > +      if (TREE_OPERAND (t, 1))
> >         /* Operand 1 is frame variable.  */
> > -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> > -      if (TREE_OPERAND (*tp, 2))
> > +       WALK_SUBTREE (TREE_OPERAND (t, 1));
> > +      if (TREE_OPERAND (t, 2))
> >         /* Operand 2 has the initialiser, and we need to walk any subtrees
> >            there.  */
> > -       WALK_SUBTREE (TREE_OPERAND (*tp, 2));
> > +       WALK_SUBTREE (TREE_OPERAND (t, 2));
> >        break;
> >
> >      case CO_RETURN_EXPR:
> > -      if (TREE_OPERAND (*tp, 0))
> > +      if (TREE_OPERAND (t, 0))
> >         {
> > -         if (VOID_TYPE_P (TREE_OPERAND (*tp, 0)))
> > +         if (VOID_TYPE_P (TREE_OPERAND (t, 0)))
> >             /* For void expressions, operand 1 is a trivial call, and any
> >                interesting subtrees will be part of operand 0.  */
> > -           WALK_SUBTREE (TREE_OPERAND (*tp, 0));
> > -         else if (TREE_OPERAND (*tp, 1))
> > +           WALK_SUBTREE (TREE_OPERAND (t, 0));
> > +         else if (TREE_OPERAND (t, 1))
> >             /* Interesting sub-trees will be in the return_value () call
> >                arguments.  */
> > -           WALK_SUBTREE (TREE_OPERAND (*tp, 1));
> > +           WALK_SUBTREE (TREE_OPERAND (t, 1));
> >         }
> >        break;
> >
> >      case STATIC_ASSERT:
> > -      WALK_SUBTREE (STATIC_ASSERT_CONDITION (*tp));
> > -      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (*tp));
> > +      WALK_SUBTREE (STATIC_ASSERT_CONDITION (t));
> > +      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (t));
> >        break;
> >
> >      default:
> > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > index 5eb6a49da31..dfada652570 100644
> > --- a/gcc/tree.cc
> > +++ b/gcc/tree.cc
> > @@ -11218,10 +11218,6 @@ tree
> >  walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >              hash_set<tree> *pset, walk_tree_lh lh)
> >  {
> > -  enum tree_code code;
> > -  int walk_subtrees;
> > -  tree result;
> > -
> >  #define WALK_SUBTREE_TAIL(NODE)                                \
> >    do                                                   \
> >      {                                                  \
> > @@ -11241,14 +11237,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >      return NULL_TREE;
> >
> >    /* Call the function.  */
> > -  walk_subtrees = 1;
> > -  result = (*func) (tp, &walk_subtrees, data);
> > +  int walk_subtrees = 1;
> > +  tree result = (*func) (tp, &walk_subtrees, data);
> >
> >    /* If we found something, return it.  */
> >    if (result)
> >      return result;
> >
> > -  code = TREE_CODE (*tp);
> > +  tree t = *tp;
> > +  tree_code code = TREE_CODE (t);
> >
> >    /* Even if we didn't, FUNC may have decided that there was nothing
> >       interesting below this point in the tree.  */
> > @@ -11256,9 +11253,9 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >      {
> >        /* But we still need to check our siblings.  */
> >        if (code == TREE_LIST)
> > -       WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
> > +       WALK_SUBTREE_TAIL (TREE_CHAIN (t));
> >        else if (code == OMP_CLAUSE)
> > -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
> > +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
> >        else
> >         return NULL_TREE;
> >      }
> > @@ -11288,58 +11285,58 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >        break;
> >
> >      case TREE_LIST:
> > -      WALK_SUBTREE (TREE_VALUE (*tp));
> > -      WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
> > +      WALK_SUBTREE (TREE_VALUE (t));
> > +      WALK_SUBTREE_TAIL (TREE_CHAIN (t));
> >
> >      case TREE_VEC:
> >        {
> > -       int len = TREE_VEC_LENGTH (*tp);
> > +       int len = TREE_VEC_LENGTH (t);
> >
> >         if (len == 0)
> >           break;
> >
> >         /* Walk all elements but the first.  */
> >         while (--len)
> > -         WALK_SUBTREE (TREE_VEC_ELT (*tp, len));
> > +         WALK_SUBTREE (TREE_VEC_ELT (t, len));
> >
> >         /* Now walk the first one as a tail call.  */
> > -       WALK_SUBTREE_TAIL (TREE_VEC_ELT (*tp, 0));
> > +       WALK_SUBTREE_TAIL (TREE_VEC_ELT (t, 0));
> >        }
> >
> >      case VECTOR_CST:
> >        {
> > -       unsigned len = vector_cst_encoded_nelts (*tp);
> > +       unsigned len = vector_cst_encoded_nelts (t);
> >         if (len == 0)
> >           break;
> >         /* Walk all elements but the first.  */
> >         while (--len)
> > -         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (*tp, len));
> > +         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (t, len));
> >         /* Now walk the first one as a tail call.  */
> > -       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (*tp, 0));
> > +       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (t, 0));
> >        }
> >
> >      case COMPLEX_CST:
> > -      WALK_SUBTREE (TREE_REALPART (*tp));
> > -      WALK_SUBTREE_TAIL (TREE_IMAGPART (*tp));
> > +      WALK_SUBTREE (TREE_REALPART (t));
> > +      WALK_SUBTREE_TAIL (TREE_IMAGPART (t));
> >
> >      case CONSTRUCTOR:
> >        {
> >         unsigned HOST_WIDE_INT idx;
> >         constructor_elt *ce;
> >
> > -       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (*tp), idx, &ce);
> > +       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (t), idx, &ce);
> >              idx++)
> >           WALK_SUBTREE (ce->value);
> >        }
> >        break;
> >
> >      case SAVE_EXPR:
> > -      WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, 0));
> > +      WALK_SUBTREE_TAIL (TREE_OPERAND (t, 0));
> >
> >      case BIND_EXPR:
> >        {
> >         tree decl;
> > -       for (decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
> > +       for (decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
> >           {
> >             /* Walk the DECL_INITIAL and DECL_SIZE.  We don't want to walk
> >                into declarations that are just mentioned, rather than
> > @@ -11350,23 +11347,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >             WALK_SUBTREE (DECL_SIZE (decl));
> >             WALK_SUBTREE (DECL_SIZE_UNIT (decl));
> >           }
> > -       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (*tp));
> > +       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (t));
> >        }
> >
> >      case STATEMENT_LIST:
> >        {
> >         tree_stmt_iterator i;
> > -       for (i = tsi_start (*tp); !tsi_end_p (i); tsi_next (&i))
> > +       for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
> >           WALK_SUBTREE (*tsi_stmt_ptr (i));
> >        }
> >        break;
> >
> >      case OMP_CLAUSE:
> >        {
> > -       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (*tp)];
> > +       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (t)];
> >         for (int i = 0; i < len; i++)
> > -         WALK_SUBTREE (OMP_CLAUSE_OPERAND (*tp, i));
> > -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
> > +         WALK_SUBTREE (OMP_CLAUSE_OPERAND (t, i));
> > +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
> >        }
> >
> >      case TARGET_EXPR:
> > @@ -11375,10 +11372,10 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >
> >         /* TARGET_EXPRs are peculiar: operands 1 and 3 can be the same.
> >            But, we only want to walk once.  */
> > -       len = (TREE_OPERAND (*tp, 3) == TREE_OPERAND (*tp, 1)) ? 2 : 3;
> > +       len = (TREE_OPERAND (t, 3) == TREE_OPERAND (t, 1)) ? 2 : 3;
> >         for (i = 0; i < len; ++i)
> > -         WALK_SUBTREE (TREE_OPERAND (*tp, i));
> > -       WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len));
> > +         WALK_SUBTREE (TREE_OPERAND (t, i));
> > +       WALK_SUBTREE_TAIL (TREE_OPERAND (t, len));
> >        }
> >
> >      case DECL_EXPR:
> > @@ -11393,15 +11390,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >          variable-sized types.
> >
> >          Note that DECLs get walked as part of processing the BIND_EXPR.  */
> > -      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL)
> > +      if (TREE_CODE (DECL_EXPR_DECL (t)) == TYPE_DECL)
> >         {
> >           /* Call the function for the decl so e.g. copy_tree_body_r can
> >              replace it with the remapped one.  */
> > -         result = (*func) (&DECL_EXPR_DECL (*tp), &walk_subtrees, data);
> > +         result = (*func) (&DECL_EXPR_DECL (t), &walk_subtrees, data);
> >           if (result || !walk_subtrees)
> >             return result;
> >
> > -         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (*tp));
> > +         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (t));
> >           if (TREE_CODE (*type_p) == ERROR_MARK)
> >             return NULL_TREE;
> >
> > @@ -11412,21 +11409,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >           if (result || !walk_subtrees)
> >             return result;
> >
> > +         tree type = *type_p;
> > +
> >           /* But do not walk a pointed-to type since it may itself need to
> >              be walked in the declaration case if it isn't anonymous.  */
> > -         if (!POINTER_TYPE_P (*type_p))
> > +         if (!POINTER_TYPE_P (type))
> >             {
> > -             result = walk_type_fields (*type_p, func, data, pset, lh);
> > +             result = walk_type_fields (type, func, data, pset, lh);
> >               if (result)
> >                 return result;
> >             }
> >
> >           /* If this is a record type, also walk the fields.  */
> > -         if (RECORD_OR_UNION_TYPE_P (*type_p))
> > +         if (RECORD_OR_UNION_TYPE_P (type))
> >             {
> >               tree field;
> >
> > -             for (field = TYPE_FIELDS (*type_p); field;
> > +             for (field = TYPE_FIELDS (type); field;
> >                    field = DECL_CHAIN (field))
> >                 {
> >                   /* We'd like to look at the type of the field, but we can
> > @@ -11439,24 +11438,24 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >                   WALK_SUBTREE (DECL_FIELD_OFFSET (field));
> >                   WALK_SUBTREE (DECL_SIZE (field));
> >                   WALK_SUBTREE (DECL_SIZE_UNIT (field));
> > -                 if (TREE_CODE (*type_p) == QUAL_UNION_TYPE)
> > +                 if (TREE_CODE (type) == QUAL_UNION_TYPE)
> >                     WALK_SUBTREE (DECL_QUALIFIER (field));
> >                 }
> >             }
> >
> >           /* Same for scalar types.  */
> > -         else if (TREE_CODE (*type_p) == BOOLEAN_TYPE
> > -                  || TREE_CODE (*type_p) == ENUMERAL_TYPE
> > -                  || TREE_CODE (*type_p) == INTEGER_TYPE
> > -                  || TREE_CODE (*type_p) == FIXED_POINT_TYPE
> > -                  || TREE_CODE (*type_p) == REAL_TYPE)
> > +         else if (TREE_CODE (type) == BOOLEAN_TYPE
> > +                  || TREE_CODE (type) == ENUMERAL_TYPE
> > +                  || TREE_CODE (type) == INTEGER_TYPE
> > +                  || TREE_CODE (type) == FIXED_POINT_TYPE
> > +                  || TREE_CODE (type) == REAL_TYPE)
> >             {
> > -             WALK_SUBTREE (TYPE_MIN_VALUE (*type_p));
> > -             WALK_SUBTREE (TYPE_MAX_VALUE (*type_p));
> > +             WALK_SUBTREE (TYPE_MIN_VALUE (type));
> > +             WALK_SUBTREE (TYPE_MAX_VALUE (type));
> >             }
> >
> > -         WALK_SUBTREE (TYPE_SIZE (*type_p));
> > -         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (*type_p));
> > +         WALK_SUBTREE (TYPE_SIZE (type));
> > +         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (type));
> >         }
> >        /* FALLTHRU */
> >
> > @@ -11466,20 +11465,20 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
> >           int i, len;
> >
> >           /* Walk over all the sub-trees of this operand.  */
> > -         len = TREE_OPERAND_LENGTH (*tp);
> > +         len = TREE_OPERAND_LENGTH (t);
> >
> >           /* Go through the subtrees.  We need to do this in forward order so
> >              that the scope of a FOR_EXPR is handled properly.  */
> >           if (len)
> >             {
> >               for (i = 0; i < len - 1; ++i)
> > -               WALK_SUBTREE (TREE_OPERAND (*tp, i));
> > -             WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len - 1));
> > +               WALK_SUBTREE (TREE_OPERAND (t, i));
> > +             WALK_SUBTREE_TAIL (TREE_OPERAND (t, len - 1));
> >             }
> >         }
> >        /* If this is a type, walk the needed fields in the type.  */
> > -      else if (TYPE_P (*tp))
> > -       return walk_type_fields (*tp, func, data, pset, lh);
> > +      else if (TYPE_P (t))
> > +       return walk_type_fields (t, func, data, pset, lh);
> >        break;
> >      }
> >
> > --
> > 2.39.0.rc0.49.g083e01275b
> >
> 
>
  

On 12/5/22 06:09, Prathamesh Kulkarni wrote:
> On Mon, 5 Dec 2022 at 09:51, Patrick Palka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> These functions currently repeatedly dereference tp during the subtree
>> walk, dereferences which the compiler can't CSE because it can't
>> guarantee that the subtree walking doesn't modify *tp.
>>
>> But we already implicitly require that TREE_CODE (*tp) remains the same
>> throughout the subtree walks, so it doesn't seem to be a huge leap to
>> strengthen that to requiring *tp remains the same.
> Hi Patrick,
> Just wondering in that case, if marking tp with const_tree *, instead
> of tree *, would perhaps help the compiler
> for CSEing some of the dereferences to *tp ?

That wouldn't make a difference; even if *tp is const, the compiler 
can't be sure that a call won't change it.  And const_tree * doesn't 
even make *tp const, it makes **tp const.

>> So this patch manually CSEs the dereferences of *tp.  This means that
>> the callback function can no longer replace *tp with another tree (of
>> the same TREE_CODE) when walking one of its subtrees, but that doesn't
>> sound like a useful feature anyway.  This speeds up the C++ frontend by
>> about ~1.5% according to my experiments.
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>> trunk stage3 or perhaps for stage1?

OK for stage 1.

>> gcc/cp/ChangeLog:
>>
>>          * tree.cc (cp_walk_subtrees): Avoid repeatedly dereferencing tp.
>>
>> gcc/ChangeLog:
>>
>>          * tree.cc (walk_tree_1): Avoid repeatedly dereferencing tp
>>          and type_p.
>> ---
>>   gcc/cp/tree.cc | 113 +++++++++++++++++++++++++------------------------
>>   gcc/tree.cc    | 103 ++++++++++++++++++++++----------------------
>>   2 files changed, 108 insertions(+), 108 deletions(-)
>>
>> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
>> index 4066b014f6e..ceb0c75f559 100644
>> --- a/gcc/cp/tree.cc
>> +++ b/gcc/cp/tree.cc
>> @@ -5441,7 +5441,8 @@ tree
>>   cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>                    void *data, hash_set<tree> *pset)
>>   {
>> -  enum tree_code code = TREE_CODE (*tp);
>> +  tree t = *tp;
>> +  enum tree_code code = TREE_CODE (t);
>>     tree result;
>>
>>   #define WALK_SUBTREE(NODE)                             \
>> @@ -5452,7 +5453,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>       }                                                  \
>>     while (0)
>>
>> -  if (TYPE_P (*tp))
>> +  if (TYPE_P (t))
>>       {
>>         /* If *WALK_SUBTREES_P is 1, we're interested in the syntactic form of
>>           the argument, so don't look through typedefs, but do walk into
>> @@ -5464,15 +5465,15 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>
>>           See find_abi_tags_r for an example of setting *WALK_SUBTREES_P to 2
>>           when that's the behavior the walk_tree_fn wants.  */
>> -      if (*walk_subtrees_p == 1 && typedef_variant_p (*tp))
>> +      if (*walk_subtrees_p == 1 && typedef_variant_p (t))
>>          {
>> -         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (*tp))
>> +         if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (t))
>>              WALK_SUBTREE (TI_ARGS (ti));
>>            *walk_subtrees_p = 0;
>>            return NULL_TREE;
>>          }
>>
>> -      if (tree ti = TYPE_TEMPLATE_INFO (*tp))
>> +      if (tree ti = TYPE_TEMPLATE_INFO (t))
>>          WALK_SUBTREE (TI_ARGS (ti));
>>       }
>>
>> @@ -5482,8 +5483,8 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>     switch (code)
>>       {
>>       case TEMPLATE_TYPE_PARM:
>> -      if (template_placeholder_p (*tp))
>> -       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (*tp));
>> +      if (template_placeholder_p (t))
>> +       WALK_SUBTREE (CLASS_PLACEHOLDER_TEMPLATE (t));
>>         /* Fall through.  */
>>       case DEFERRED_PARSE:
>>       case TEMPLATE_TEMPLATE_PARM:
>> @@ -5497,63 +5498,63 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>         break;
>>
>>       case TYPENAME_TYPE:
>> -      WALK_SUBTREE (TYPE_CONTEXT (*tp));
>> -      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (*tp));
>> +      WALK_SUBTREE (TYPE_CONTEXT (t));
>> +      WALK_SUBTREE (TYPENAME_TYPE_FULLNAME (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case BASELINK:
>> -      if (BASELINK_QUALIFIED_P (*tp))
>> -       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (*tp)));
>> -      WALK_SUBTREE (BASELINK_FUNCTIONS (*tp));
>> +      if (BASELINK_QUALIFIED_P (t))
>> +       WALK_SUBTREE (BINFO_TYPE (BASELINK_ACCESS_BINFO (t)));
>> +      WALK_SUBTREE (BASELINK_FUNCTIONS (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case PTRMEM_CST:
>> -      WALK_SUBTREE (TREE_TYPE (*tp));
>> +      WALK_SUBTREE (TREE_TYPE (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case TREE_LIST:
>> -      WALK_SUBTREE (TREE_PURPOSE (*tp));
>> +      WALK_SUBTREE (TREE_PURPOSE (t));
>>         break;
>>
>>       case OVERLOAD:
>> -      WALK_SUBTREE (OVL_FUNCTION (*tp));
>> -      WALK_SUBTREE (OVL_CHAIN (*tp));
>> +      WALK_SUBTREE (OVL_FUNCTION (t));
>> +      WALK_SUBTREE (OVL_CHAIN (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case USING_DECL:
>> -      WALK_SUBTREE (DECL_NAME (*tp));
>> -      WALK_SUBTREE (USING_DECL_SCOPE (*tp));
>> -      WALK_SUBTREE (USING_DECL_DECLS (*tp));
>> +      WALK_SUBTREE (DECL_NAME (t));
>> +      WALK_SUBTREE (USING_DECL_SCOPE (t));
>> +      WALK_SUBTREE (USING_DECL_DECLS (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case RECORD_TYPE:
>> -      if (TYPE_PTRMEMFUNC_P (*tp))
>> -       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (*tp));
>> +      if (TYPE_PTRMEMFUNC_P (t))
>> +       WALK_SUBTREE (TYPE_PTRMEMFUNC_FN_TYPE_RAW (t));
>>         break;
>>
>>       case TYPE_ARGUMENT_PACK:
>>       case NONTYPE_ARGUMENT_PACK:
>>         {
>> -        tree args = ARGUMENT_PACK_ARGS (*tp);
>> +       tree args = ARGUMENT_PACK_ARGS (t);
>>          for (tree arg : tree_vec_range (args))
>>            WALK_SUBTREE (arg);
>>         }
>>         break;
>>
>>       case TYPE_PACK_EXPANSION:
>> -      WALK_SUBTREE (TREE_TYPE (*tp));
>> -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
>> +      WALK_SUBTREE (TREE_TYPE (t));
>> +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case EXPR_PACK_EXPANSION:
>> -      WALK_SUBTREE (TREE_OPERAND (*tp, 0));
>> -      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (*tp));
>> +      WALK_SUBTREE (TREE_OPERAND (t, 0));
>> +      WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>> @@ -5564,31 +5565,31 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>       case DYNAMIC_CAST_EXPR:
>>       case IMPLICIT_CONV_EXPR:
>>       case BIT_CAST_EXPR:
>> -      if (TREE_TYPE (*tp))
>> -       WALK_SUBTREE (TREE_TYPE (*tp));
>> +      if (TREE_TYPE (t))
>> +       WALK_SUBTREE (TREE_TYPE (t));
>>         break;
>>
>>       case CONSTRUCTOR:
>> -      if (COMPOUND_LITERAL_P (*tp))
>> -       WALK_SUBTREE (TREE_TYPE (*tp));
>> +      if (COMPOUND_LITERAL_P (t))
>> +       WALK_SUBTREE (TREE_TYPE (t));
>>         break;
>>
>>       case TRAIT_EXPR:
>> -      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (*tp));
>> -      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (*tp));
>> +      WALK_SUBTREE (TRAIT_EXPR_TYPE1 (t));
>> +      WALK_SUBTREE (TRAIT_EXPR_TYPE2 (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case TRAIT_TYPE:
>> -      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (*tp));
>> -      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (*tp));
>> +      WALK_SUBTREE (TRAIT_TYPE_TYPE1 (t));
>> +      WALK_SUBTREE (TRAIT_TYPE_TYPE2 (t));
>>         *walk_subtrees_p = 0;
>>         break;
>>
>>       case DECLTYPE_TYPE:
>>         ++cp_unevaluated_operand;
>>         /* We can't use WALK_SUBTREE here because of the goto.  */
>> -      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (*tp), func, data, pset);
>> +      result = cp_walk_tree (&DECLTYPE_TYPE_EXPR (t), func, data, pset);
>>         --cp_unevaluated_operand;
>>         *walk_subtrees_p = 0;
>>         break;
>> @@ -5597,7 +5598,7 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>       case SIZEOF_EXPR:
>>       case NOEXCEPT_EXPR:
>>         ++cp_unevaluated_operand;
>> -      result = cp_walk_tree (&TREE_OPERAND (*tp, 0), func, data, pset);
>> +      result = cp_walk_tree (&TREE_OPERAND (t, 0), func, data, pset);
>>         --cp_unevaluated_operand;
>>         *walk_subtrees_p = 0;
>>         break;
>> @@ -5605,12 +5606,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>       case REQUIRES_EXPR:
>>         {
>>          cp_unevaluated u;
>> -       for (tree parm = REQUIRES_EXPR_PARMS (*tp); parm; parm = DECL_CHAIN (parm))
>> +       for (tree parm = REQUIRES_EXPR_PARMS (t); parm; parm = DECL_CHAIN (parm))
>>            /* Walk the types of each parameter, but not the parameter itself.
>>               Doing so would cause false positives in the unexpanded parameter
>>               pack checker since an introduced parameter pack looks bare.  */
>>            WALK_SUBTREE (TREE_TYPE (parm));
>> -       WALK_SUBTREE (REQUIRES_EXPR_REQS (*tp));
>> +       WALK_SUBTREE (REQUIRES_EXPR_REQS (t));
>>          *walk_subtrees_p = 0;
>>          break;
>>         }
>> @@ -5621,12 +5622,12 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>           the containing BIND_EXPR.  Compiler temporaries are
>>           handled here.  And also normal variables in templates,
>>           since do_poplevel doesn't build a BIND_EXPR then.  */
>> -      if (VAR_P (TREE_OPERAND (*tp, 0))
>> +      if (VAR_P (TREE_OPERAND (t, 0))
>>            && (processing_template_decl
>> -             || (DECL_ARTIFICIAL (TREE_OPERAND (*tp, 0))
>> -                 && !TREE_STATIC (TREE_OPERAND (*tp, 0)))))
>> +             || (DECL_ARTIFICIAL (TREE_OPERAND (t, 0))
>> +                 && !TREE_STATIC (TREE_OPERAND (t, 0)))))
>>          {
>> -         tree decl = TREE_OPERAND (*tp, 0);
>> +         tree decl = TREE_OPERAND (t, 0);
>>            WALK_SUBTREE (DECL_INITIAL (decl));
>>            WALK_SUBTREE (DECL_SIZE (decl));
>>            WALK_SUBTREE (DECL_SIZE_UNIT (decl));
>> @@ -5636,45 +5637,45 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>>       case LAMBDA_EXPR:
>>         /* Don't walk into the body of the lambda, but the capture initializers
>>           are part of the enclosing context.  */
>> -      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (*tp); cap;
>> +      for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (t); cap;
>>             cap = TREE_CHAIN (cap))
>>          WALK_SUBTREE (TREE_VALUE (cap));
>>         break;
>>
>>       case CO_YIELD_EXPR:
>> -      if (TREE_OPERAND (*tp, 1))
>> +      if (TREE_OPERAND (t, 1))
>>          /* Operand 1 is the tree for the relevant co_await which has any
>>             interesting sub-trees.  */
>> -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
>> +       WALK_SUBTREE (TREE_OPERAND (t, 1));
>>         break;
>>
>>       case CO_AWAIT_EXPR:
>> -      if (TREE_OPERAND (*tp, 1))
>> +      if (TREE_OPERAND (t, 1))
>>          /* Operand 1 is frame variable.  */
>> -       WALK_SUBTREE (TREE_OPERAND (*tp, 1));
>> -      if (TREE_OPERAND (*tp, 2))
>> +       WALK_SUBTREE (TREE_OPERAND (t, 1));
>> +      if (TREE_OPERAND (t, 2))
>>          /* Operand 2 has the initialiser, and we need to walk any subtrees
>>             there.  */
>> -       WALK_SUBTREE (TREE_OPERAND (*tp, 2));
>> +       WALK_SUBTREE (TREE_OPERAND (t, 2));
>>         break;
>>
>>       case CO_RETURN_EXPR:
>> -      if (TREE_OPERAND (*tp, 0))
>> +      if (TREE_OPERAND (t, 0))
>>          {
>> -         if (VOID_TYPE_P (TREE_OPERAND (*tp, 0)))
>> +         if (VOID_TYPE_P (TREE_OPERAND (t, 0)))
>>              /* For void expressions, operand 1 is a trivial call, and any
>>                 interesting subtrees will be part of operand 0.  */
>> -           WALK_SUBTREE (TREE_OPERAND (*tp, 0));
>> -         else if (TREE_OPERAND (*tp, 1))
>> +           WALK_SUBTREE (TREE_OPERAND (t, 0));
>> +         else if (TREE_OPERAND (t, 1))
>>              /* Interesting sub-trees will be in the return_value () call
>>                 arguments.  */
>> -           WALK_SUBTREE (TREE_OPERAND (*tp, 1));
>> +           WALK_SUBTREE (TREE_OPERAND (t, 1));
>>          }
>>         break;
>>
>>       case STATIC_ASSERT:
>> -      WALK_SUBTREE (STATIC_ASSERT_CONDITION (*tp));
>> -      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (*tp));
>> +      WALK_SUBTREE (STATIC_ASSERT_CONDITION (t));
>> +      WALK_SUBTREE (STATIC_ASSERT_MESSAGE (t));
>>         break;
>>
>>       default:
>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>> index 5eb6a49da31..dfada652570 100644
>> --- a/gcc/tree.cc
>> +++ b/gcc/tree.cc
>> @@ -11218,10 +11218,6 @@ tree
>>   walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>               hash_set<tree> *pset, walk_tree_lh lh)
>>   {
>> -  enum tree_code code;
>> -  int walk_subtrees;
>> -  tree result;
>> -
>>   #define WALK_SUBTREE_TAIL(NODE)                                \
>>     do                                                   \
>>       {                                                  \
>> @@ -11241,14 +11237,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>       return NULL_TREE;
>>
>>     /* Call the function.  */
>> -  walk_subtrees = 1;
>> -  result = (*func) (tp, &walk_subtrees, data);
>> +  int walk_subtrees = 1;
>> +  tree result = (*func) (tp, &walk_subtrees, data);
>>
>>     /* If we found something, return it.  */
>>     if (result)
>>       return result;
>>
>> -  code = TREE_CODE (*tp);
>> +  tree t = *tp;
>> +  tree_code code = TREE_CODE (t);
>>
>>     /* Even if we didn't, FUNC may have decided that there was nothing
>>        interesting below this point in the tree.  */
>> @@ -11256,9 +11253,9 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>       {
>>         /* But we still need to check our siblings.  */
>>         if (code == TREE_LIST)
>> -       WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
>> +       WALK_SUBTREE_TAIL (TREE_CHAIN (t));
>>         else if (code == OMP_CLAUSE)
>> -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
>> +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
>>         else
>>          return NULL_TREE;
>>       }
>> @@ -11288,58 +11285,58 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>         break;
>>
>>       case TREE_LIST:
>> -      WALK_SUBTREE (TREE_VALUE (*tp));
>> -      WALK_SUBTREE_TAIL (TREE_CHAIN (*tp));
>> +      WALK_SUBTREE (TREE_VALUE (t));
>> +      WALK_SUBTREE_TAIL (TREE_CHAIN (t));
>>
>>       case TREE_VEC:
>>         {
>> -       int len = TREE_VEC_LENGTH (*tp);
>> +       int len = TREE_VEC_LENGTH (t);
>>
>>          if (len == 0)
>>            break;
>>
>>          /* Walk all elements but the first.  */
>>          while (--len)
>> -         WALK_SUBTREE (TREE_VEC_ELT (*tp, len));
>> +         WALK_SUBTREE (TREE_VEC_ELT (t, len));
>>
>>          /* Now walk the first one as a tail call.  */
>> -       WALK_SUBTREE_TAIL (TREE_VEC_ELT (*tp, 0));
>> +       WALK_SUBTREE_TAIL (TREE_VEC_ELT (t, 0));
>>         }
>>
>>       case VECTOR_CST:
>>         {
>> -       unsigned len = vector_cst_encoded_nelts (*tp);
>> +       unsigned len = vector_cst_encoded_nelts (t);
>>          if (len == 0)
>>            break;
>>          /* Walk all elements but the first.  */
>>          while (--len)
>> -         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (*tp, len));
>> +         WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (t, len));
>>          /* Now walk the first one as a tail call.  */
>> -       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (*tp, 0));
>> +       WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (t, 0));
>>         }
>>
>>       case COMPLEX_CST:
>> -      WALK_SUBTREE (TREE_REALPART (*tp));
>> -      WALK_SUBTREE_TAIL (TREE_IMAGPART (*tp));
>> +      WALK_SUBTREE (TREE_REALPART (t));
>> +      WALK_SUBTREE_TAIL (TREE_IMAGPART (t));
>>
>>       case CONSTRUCTOR:
>>         {
>>          unsigned HOST_WIDE_INT idx;
>>          constructor_elt *ce;
>>
>> -       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (*tp), idx, &ce);
>> +       for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (t), idx, &ce);
>>               idx++)
>>            WALK_SUBTREE (ce->value);
>>         }
>>         break;
>>
>>       case SAVE_EXPR:
>> -      WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, 0));
>> +      WALK_SUBTREE_TAIL (TREE_OPERAND (t, 0));
>>
>>       case BIND_EXPR:
>>         {
>>          tree decl;
>> -       for (decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
>> +       for (decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
>>            {
>>              /* Walk the DECL_INITIAL and DECL_SIZE.  We don't want to walk
>>                 into declarations that are just mentioned, rather than
>> @@ -11350,23 +11347,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>              WALK_SUBTREE (DECL_SIZE (decl));
>>              WALK_SUBTREE (DECL_SIZE_UNIT (decl));
>>            }
>> -       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (*tp));
>> +       WALK_SUBTREE_TAIL (BIND_EXPR_BODY (t));
>>         }
>>
>>       case STATEMENT_LIST:
>>         {
>>          tree_stmt_iterator i;
>> -       for (i = tsi_start (*tp); !tsi_end_p (i); tsi_next (&i))
>> +       for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
>>            WALK_SUBTREE (*tsi_stmt_ptr (i));
>>         }
>>         break;
>>
>>       case OMP_CLAUSE:
>>         {
>> -       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (*tp)];
>> +       int len = omp_clause_num_ops[OMP_CLAUSE_CODE (t)];
>>          for (int i = 0; i < len; i++)
>> -         WALK_SUBTREE (OMP_CLAUSE_OPERAND (*tp, i));
>> -       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
>> +         WALK_SUBTREE (OMP_CLAUSE_OPERAND (t, i));
>> +       WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (t));
>>         }
>>
>>       case TARGET_EXPR:
>> @@ -11375,10 +11372,10 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>
>>          /* TARGET_EXPRs are peculiar: operands 1 and 3 can be the same.
>>             But, we only want to walk once.  */
>> -       len = (TREE_OPERAND (*tp, 3) == TREE_OPERAND (*tp, 1)) ? 2 : 3;
>> +       len = (TREE_OPERAND (t, 3) == TREE_OPERAND (t, 1)) ? 2 : 3;
>>          for (i = 0; i < len; ++i)
>> -         WALK_SUBTREE (TREE_OPERAND (*tp, i));
>> -       WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len));
>> +         WALK_SUBTREE (TREE_OPERAND (t, i));
>> +       WALK_SUBTREE_TAIL (TREE_OPERAND (t, len));
>>         }
>>
>>       case DECL_EXPR:
>> @@ -11393,15 +11390,15 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>           variable-sized types.
>>
>>           Note that DECLs get walked as part of processing the BIND_EXPR.  */
>> -      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL)
>> +      if (TREE_CODE (DECL_EXPR_DECL (t)) == TYPE_DECL)
>>          {
>>            /* Call the function for the decl so e.g. copy_tree_body_r can
>>               replace it with the remapped one.  */
>> -         result = (*func) (&DECL_EXPR_DECL (*tp), &walk_subtrees, data);
>> +         result = (*func) (&DECL_EXPR_DECL (t), &walk_subtrees, data);
>>            if (result || !walk_subtrees)
>>              return result;
>>
>> -         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (*tp));
>> +         tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (t));
>>            if (TREE_CODE (*type_p) == ERROR_MARK)
>>              return NULL_TREE;
>>
>> @@ -11412,21 +11409,23 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>            if (result || !walk_subtrees)
>>              return result;
>>
>> +         tree type = *type_p;
>> +
>>            /* But do not walk a pointed-to type since it may itself need to
>>               be walked in the declaration case if it isn't anonymous.  */
>> -         if (!POINTER_TYPE_P (*type_p))
>> +         if (!POINTER_TYPE_P (type))
>>              {
>> -             result = walk_type_fields (*type_p, func, data, pset, lh);
>> +             result = walk_type_fields (type, func, data, pset, lh);
>>                if (result)
>>                  return result;
>>              }
>>
>>            /* If this is a record type, also walk the fields.  */
>> -         if (RECORD_OR_UNION_TYPE_P (*type_p))
>> +         if (RECORD_OR_UNION_TYPE_P (type))
>>              {
>>                tree field;
>>
>> -             for (field = TYPE_FIELDS (*type_p); field;
>> +             for (field = TYPE_FIELDS (type); field;
>>                     field = DECL_CHAIN (field))
>>                  {
>>                    /* We'd like to look at the type of the field, but we can
>> @@ -11439,24 +11438,24 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>                    WALK_SUBTREE (DECL_FIELD_OFFSET (field));
>>                    WALK_SUBTREE (DECL_SIZE (field));
>>                    WALK_SUBTREE (DECL_SIZE_UNIT (field));
>> -                 if (TREE_CODE (*type_p) == QUAL_UNION_TYPE)
>> +                 if (TREE_CODE (type) == QUAL_UNION_TYPE)
>>                      WALK_SUBTREE (DECL_QUALIFIER (field));
>>                  }
>>              }
>>
>>            /* Same for scalar types.  */
>> -         else if (TREE_CODE (*type_p) == BOOLEAN_TYPE
>> -                  || TREE_CODE (*type_p) == ENUMERAL_TYPE
>> -                  || TREE_CODE (*type_p) == INTEGER_TYPE
>> -                  || TREE_CODE (*type_p) == FIXED_POINT_TYPE
>> -                  || TREE_CODE (*type_p) == REAL_TYPE)
>> +         else if (TREE_CODE (type) == BOOLEAN_TYPE
>> +                  || TREE_CODE (type) == ENUMERAL_TYPE
>> +                  || TREE_CODE (type) == INTEGER_TYPE
>> +                  || TREE_CODE (type) == FIXED_POINT_TYPE
>> +                  || TREE_CODE (type) == REAL_TYPE)
>>              {
>> -             WALK_SUBTREE (TYPE_MIN_VALUE (*type_p));
>> -             WALK_SUBTREE (TYPE_MAX_VALUE (*type_p));
>> +             WALK_SUBTREE (TYPE_MIN_VALUE (type));
>> +             WALK_SUBTREE (TYPE_MAX_VALUE (type));
>>              }
>>
>> -         WALK_SUBTREE (TYPE_SIZE (*type_p));
>> -         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (*type_p));
>> +         WALK_SUBTREE (TYPE_SIZE (type));
>> +         WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (type));
>>          }
>>         /* FALLTHRU */
>>
>> @@ -11466,20 +11465,20 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
>>            int i, len;
>>
>>            /* Walk over all the sub-trees of this operand.  */
>> -         len = TREE_OPERAND_LENGTH (*tp);
>> +         len = TREE_OPERAND_LENGTH (t);
>>
>>            /* Go through the subtrees.  We need to do this in forward order so
>>               that the scope of a FOR_EXPR is handled properly.  */
>>            if (len)
>>              {
>>                for (i = 0; i < len - 1; ++i)
>> -               WALK_SUBTREE (TREE_OPERAND (*tp, i));
>> -             WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len - 1));
>> +               WALK_SUBTREE (TREE_OPERAND (t, i));
>> +             WALK_SUBTREE_TAIL (TREE_OPERAND (t, len - 1));
>>              }
>>          }
>>         /* If this is a type, walk the needed fields in the type.  */
>> -      else if (TYPE_P (*tp))
>> -       return walk_type_fields (*tp, func, data, pset, lh);
>> +      else if (TYPE_P (t))
>> +       return walk_type_fields (t, func, data, pset, lh);
>>         break;
>>       }
>>
>> --
>> 2.39.0.rc0.49.g083e01275b
>>
>