public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Andrew Pinski <apinski@marvell.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 1/2] Add flow_sensitive_info_storage and use it in gimple-fold.
Date: Wed, 19 Jul 2023 10:32:37 +0200	[thread overview]
Message-ID: <CAFiYyc0peD5jaCbn+HVzELqjV2WHys_OsDZp-Zpskic92oww9g@mail.gmail.com> (raw)
In-Reply-To: <20230715031957.1147225-1-apinski@marvell.com>

On Sat, Jul 15, 2023 at 5:21 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This adds flow_sensitive_info_storage and uses it in
> maybe_fold_comparisons_from_match_pd as mentioned in
> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621817.html .
> Since using it in maybe_fold_comparisons_from_match_pd was easy
> and allowed me to test the storage earlier, I did it.
>
> This also hides better how the flow sensitive information is
> stored and only a single place needs to be updated if that
> ever changes (again).
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

Thanks for doing this!
Richard.

> gcc/ChangeLog:
>
>         * gimple-fold.cc (fosa_unwind): Replace `vrange_storage *`
>         with flow_sensitive_info_storage.
>         (follow_outer_ssa_edges): Update how to save off the flow
>         sensitive info.
>         (maybe_fold_comparisons_from_match_pd): Update restoring
>         of flow sensitive info.
>         * tree-ssanames.cc (flow_sensitive_info_storage::save): New method.
>         (flow_sensitive_info_storage::restore): New method.
>         (flow_sensitive_info_storage::save_and_clear): New method.
>         (flow_sensitive_info_storage::clear_storage): New method.
>         * tree-ssanames.h (class flow_sensitive_info_storage): New class.
> ---
>  gcc/gimple-fold.cc   | 17 +++++------
>  gcc/tree-ssanames.cc | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  gcc/tree-ssanames.h  | 21 +++++++++++++
>  3 files changed, 100 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 4027ff71e10..de94efbcff7 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -6947,7 +6947,7 @@ and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b,
>  }
>
>  static basic_block fosa_bb;
> -static vec<std::pair<tree, vrange_storage *> > *fosa_unwind;
> +static vec<std::pair<tree, flow_sensitive_info_storage> > *fosa_unwind;
>  static tree
>  follow_outer_ssa_edges (tree val)
>  {
> @@ -6967,14 +6967,11 @@ follow_outer_ssa_edges (tree val)
>            || POINTER_TYPE_P (TREE_TYPE (val)))
>           && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (val)))
>         return NULL_TREE;
> +      flow_sensitive_info_storage storage;
> +      storage.save_and_clear (val);
>        /* If the definition does not dominate fosa_bb temporarily reset
>          flow-sensitive info.  */
> -      if (val->ssa_name.info.range_info)
> -       {
> -         fosa_unwind->safe_push (std::make_pair
> -                                   (val, val->ssa_name.info.range_info));
> -         val->ssa_name.info.range_info = NULL;
> -       }
> +      fosa_unwind->safe_push (std::make_pair (val, storage));
>        return val;
>      }
>    return val;
> @@ -7034,14 +7031,14 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code,
>                       type, gimple_assign_lhs (stmt1),
>                       gimple_assign_lhs (stmt2));
>    fosa_bb = outer_cond_bb;
> -  auto_vec<std::pair<tree, vrange_storage *>, 8> unwind_stack;
> +  auto_vec<std::pair<tree, flow_sensitive_info_storage>, 8> unwind_stack;
>    fosa_unwind = &unwind_stack;
>    if (op.resimplify (NULL, (!outer_cond_bb
>                             ? follow_all_ssa_edges : follow_outer_ssa_edges)))
>      {
>        fosa_unwind = NULL;
>        for (auto p : unwind_stack)
> -       p.first->ssa_name.info.range_info = p.second;
> +       p.second.restore (p.first);
>        if (gimple_simplified_result_is_gimple_val (&op))
>         {
>           tree res = op.ops[0];
> @@ -7065,7 +7062,7 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code,
>      }
>    fosa_unwind = NULL;
>    for (auto p : unwind_stack)
> -    p.first->ssa_name.info.range_info = p.second;
> +    p.second.restore (p.first);
>
>    return NULL_TREE;
>  }
> diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
> index 5fdb6a37e9f..f81332451fc 100644
> --- a/gcc/tree-ssanames.cc
> +++ b/gcc/tree-ssanames.cc
> @@ -916,3 +916,75 @@ make_pass_release_ssa_names (gcc::context *ctxt)
>  {
>    return new pass_release_ssa_names (ctxt);
>  }
> +
> +/* Save and restore of flow sensitive information. */
> +
> +/* Save off the flow sensitive info from NAME. */
> +
> +void
> +flow_sensitive_info_storage::save (tree name)
> +{
> +  gcc_assert (state == 0);
> +  if (!POINTER_TYPE_P (TREE_TYPE (name)))
> +    {
> +      range_info = SSA_NAME_RANGE_INFO (name);
> +      state = 1;
> +      return;
> +    }
> +  state = -1;
> +  auto ptr_info = SSA_NAME_PTR_INFO (name);
> +  if (ptr_info)
> +    {
> +      align = ptr_info->align;
> +      misalign = ptr_info->misalign;
> +      null = SSA_NAME_PTR_INFO (name)->pt.null;
> +    }
> +  else
> +    {
> +      align = 0;
> +      misalign = 0;
> +      null = true;
> +    }
> +}
> +
> +/* Restore the flow sensitive info from NAME. */
> +
> +void
> +flow_sensitive_info_storage::restore (tree name)
> +{
> +  gcc_assert (state != 0);
> +  if (!POINTER_TYPE_P (TREE_TYPE (name)))
> +    {
> +      gcc_assert (state == 1);
> +      SSA_NAME_RANGE_INFO (name) = range_info;
> +      return;
> +    }
> +  gcc_assert (state == -1);
> +  auto ptr_info = SSA_NAME_PTR_INFO (name);
> +  /* If there was no flow sensitive info on the pointer
> +     just return, there is nothing to restore to.  */
> +  if (!ptr_info)
> +    return;
> +  if (align != 0)
> +    set_ptr_info_alignment (ptr_info, align, misalign);
> +  else
> +    mark_ptr_info_alignment_unknown (ptr_info);
> +  SSA_NAME_PTR_INFO (name)->pt.null = null;
> +}
> +
> +/* Save off the flow sensitive info from NAME.
> +   And reset the flow sensitive info of NAME. */
> +
> +void
> +flow_sensitive_info_storage::save_and_clear (tree name)
> +{
> +  save (name);
> +  reset_flow_sensitive_info (name);
> +}
> +
> +/* Clear the storage. */
> +void
> +flow_sensitive_info_storage::clear_storage (void)
> +{
> +  state = 0;
> +}
> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
> index f3fa609208a..98f4a835483 100644
> --- a/gcc/tree-ssanames.h
> +++ b/gcc/tree-ssanames.h
> @@ -136,5 +136,26 @@ make_temp_ssa_name (tree type, gimple *stmt, const char *name)
>    return ssa_name;
>  }
>
> +/* A class which is used to save/restore the flow sensitive information.  */
> +class flow_sensitive_info_storage
> +{
> +public:
> +  void save (tree);
> +  void save_and_clear (tree);
> +  void restore (tree);
> +  void clear_storage ();
> +private:
> +  /* 0 means there is nothing saved.
> +     1 means non pointer is saved.
> +     -1 means a pointer type is saved.
> +     -2 means a pointer type is saved but no information was saved. */
> +  int state = 0;
> +  /* The range info for non pointers */
> +  vrange_storage *range_info = nullptr;
> +  /* Flow sensitive pointer information. */
> +  unsigned int align = 0;
> +  unsigned int misalign = 0;
> +  bool null = true;
> +};
>
>  #endif /* GCC_TREE_SSANAMES_H */
> --
> 2.31.1
>

      parent reply	other threads:[~2023-07-19  8:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-15  3:19 Andrew Pinski
2023-07-15  3:19 ` [PATCH 2/2] [PATCH] Fix tree-opt/110252: wrong code due to phiopt using flow sensitive info during match Andrew Pinski
2023-07-19  8:33   ` Richard Biener
2023-07-19  8:32 ` Richard Biener [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFiYyc0peD5jaCbn+HVzELqjV2WHys_OsDZp-Zpskic92oww9g@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=apinski@marvell.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).