[v2] Canonicalize &MEM[ssa_n, CST] to ssa_n p+ CST in fold_stmt_1

Message ID 1637645378-8431-1-git-send-email-apinski@marvell.com
State New
Headers
Series [v2] Canonicalize &MEM[ssa_n, CST] to ssa_n p+ CST in fold_stmt_1 |

Commit Message

Li, Pan2 via Gcc-patches Nov. 23, 2021, 5:29 a.m. UTC
  From: Andrew Pinski <apinski@marvell.com>

This is a new version of the patch to fix PR 102216.
Instead of doing the canonicalization inside forwprop, Richi
mentioned we should do it inside fold_stmt_1 and that is what
this patch does.

	PR tree-optimization/102216

gcc/ChangeLog:

	* gimple-fold.c (fold_stmt_1): Add canonicalization
	of "&MEM[ssa_n, CST]" to "ssa_n p+ CST", note this
	can only be done if !in_place.

gcc/testsuite/ChangeLog:

	* g++.dg/tree-ssa/pr102216-1.C: New test.
	* g++.dg/tree-ssa/pr102216-2.C: New test.
---
 gcc/gimple-fold.c                          | 21 ++++++++++
 gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C | 21 ++++++++++
 gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C | 45 ++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
  

Comments

Richard Biener Nov. 23, 2021, 9:36 a.m. UTC | #1
On Tue, Nov 23, 2021 at 6:30 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> This is a new version of the patch to fix PR 102216.
> Instead of doing the canonicalization inside forwprop, Richi
> mentioned we should do it inside fold_stmt_1 and that is what
> this patch does.
>
>         PR tree-optimization/102216
>
> gcc/ChangeLog:
>
>         * gimple-fold.c (fold_stmt_1): Add canonicalization
>         of "&MEM[ssa_n, CST]" to "ssa_n p+ CST", note this
>         can only be done if !in_place.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/tree-ssa/pr102216-1.C: New test.
>         * g++.dg/tree-ssa/pr102216-2.C: New test.
> ---
>  gcc/gimple-fold.c                          | 21 ++++++++++
>  gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C | 21 ++++++++++
>  gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C | 45 ++++++++++++++++++++++
>  3 files changed, 87 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
>
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index ad9703ee471..aab6818c93f 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -6061,6 +6061,27 @@ fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree))
>           if (REFERENCE_CLASS_P (*lhs)
>               && maybe_canonicalize_mem_ref_addr (lhs))
>             changed = true;
> +         /* Canonicalize &MEM[ssa_n, CST] to ssa_n p+ CST.
> +            This cannot be done in maybe_canonicalize_mem_ref_addr
> +            as the gimple now has two operands rather than one.
> +            The same reason why this can't be done in
> +            maybe_canonicalize_mem_ref_addr is the same reason why
> +            this can't be done inplace.  */
> +         if (!inplace && TREE_CODE (*rhs) == ADDR_EXPR)
> +           {
> +             tree inner = TREE_OPERAND (*rhs, 0);
> +             if (TREE_CODE (inner) == MEM_REF
> +                 && TREE_CODE (TREE_OPERAND (inner, 0)) == SSA_NAME

fold_stmt also works pre-SSA, so instead check for != ADDR_EXPR here

> +                 && TREE_CODE (TREE_OPERAND (inner, 1)) == INTEGER_CST)
> +               {
> +                 tree ptr = TREE_OPERAND (inner, 0);
> +                 tree addon = TREE_OPERAND (inner, 1);
> +                 addon = fold_convert (sizetype, addon);
> +                 gimple_assign_set_rhs_with_ops (gsi, POINTER_PLUS_EXPR,
> +                                                 ptr, addon);

please update 'stmt' here from gsi

OK with those changes.

Thanks,
Richard.

> +                 changed = true;
> +               }
> +           }
>         }
>        else
>         {
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C b/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
> new file mode 100644
> index 00000000000..21f7f6797ff
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
> @@ -0,0 +1,21 @@
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +void link_error ();
> +void g ()
> +{
> +  const char **language_names;
> +
> +  language_names = new const char *[6];
> +
> +  const char **language_names_p = language_names;
> +
> +  language_names_p++;
> +  language_names_p++;
> +  language_names_p++;
> +
> +  if ( (language_names_p) - (language_names+3) != 0)
> +    link_error();
> +  delete[] language_names;
> +}
> +/* We should have removed the link_error on the gimple level as GCC should
> +   be able to tell that language_names_p is the same as language_names+3.  */
> +/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C b/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
> new file mode 100644
> index 00000000000..8d351a9bad0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
> @@ -0,0 +1,45 @@
> +/* { dg-options "-O2 -Wall" } */
> +#include <algorithm>
> +
> +static inline bool
> +compare_cstrings (const char *str1, const char *str2)
> +{
> +  return str1 < str2;
> +}
> +
> +void
> +add_set_language_command ()
> +{
> +  static const char **language_names;
> +
> +  language_names = new const char *[6];
> +
> +  language_names[0] = "auto";
> +  language_names[1] = "local";
> +  language_names[2] = "unknown";
> +
> +  const char **language_names_p = language_names;
> +  /* language_names_p == &language_names[0].  */
> +  language_names_p++;
> +  /* language_names_p == &language_names[1].  */
> +  language_names_p++;
> +  /* language_names_p == &language_names[2].  */
> +  language_names_p++;
> +  /* language_names_p == &language_names[3].  */
> +
> +  const char **sort_begin;
> +
> +  if (0)
> +    sort_begin = &language_names[3];
> +  else
> +    sort_begin = language_names_p;
> +
> +  language_names[3] = "";
> +  language_names[4] = "";
> +  language_names[5] = NULL;
> +
> +  /* There should be no warning associated with this std::sort as
> +     sort_begin != &language_names[5] and GCC should be able to figure
> +     that out.  */
> +  std::sort (sort_begin, &language_names[5], compare_cstrings);
> +}
> --
> 2.17.1
>
  

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index ad9703ee471..aab6818c93f 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -6061,6 +6061,27 @@  fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree))
 	  if (REFERENCE_CLASS_P (*lhs)
 	      && maybe_canonicalize_mem_ref_addr (lhs))
 	    changed = true;
+	  /* Canonicalize &MEM[ssa_n, CST] to ssa_n p+ CST.
+	     This cannot be done in maybe_canonicalize_mem_ref_addr
+	     as the gimple now has two operands rather than one.
+	     The same reason why this can't be done in
+	     maybe_canonicalize_mem_ref_addr is the same reason why
+	     this can't be done inplace.  */
+	  if (!inplace && TREE_CODE (*rhs) == ADDR_EXPR)
+	    {
+	      tree inner = TREE_OPERAND (*rhs, 0);
+	      if (TREE_CODE (inner) == MEM_REF
+		  && TREE_CODE (TREE_OPERAND (inner, 0)) == SSA_NAME
+		  && TREE_CODE (TREE_OPERAND (inner, 1)) == INTEGER_CST)
+		{
+		  tree ptr = TREE_OPERAND (inner, 0);
+		  tree addon = TREE_OPERAND (inner, 1);
+		  addon = fold_convert (sizetype, addon);
+		  gimple_assign_set_rhs_with_ops (gsi, POINTER_PLUS_EXPR,
+						  ptr, addon);
+		  changed = true;
+		}
+	    }
 	}
       else
 	{
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C b/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
new file mode 100644
index 00000000000..21f7f6797ff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216-1.C
@@ -0,0 +1,21 @@ 
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+void link_error ();
+void g ()
+{
+  const char **language_names;
+
+  language_names = new const char *[6];
+
+  const char **language_names_p = language_names;
+
+  language_names_p++;
+  language_names_p++;
+  language_names_p++;
+
+  if ( (language_names_p) - (language_names+3) != 0)
+    link_error();
+  delete[] language_names;
+}
+/* We should have removed the link_error on the gimple level as GCC should
+   be able to tell that language_names_p is the same as language_names+3.  */
+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C b/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
new file mode 100644
index 00000000000..8d351a9bad0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216-2.C
@@ -0,0 +1,45 @@ 
+/* { dg-options "-O2 -Wall" } */
+#include <algorithm>
+
+static inline bool
+compare_cstrings (const char *str1, const char *str2)
+{
+  return str1 < str2;
+}
+
+void
+add_set_language_command ()
+{
+  static const char **language_names;
+
+  language_names = new const char *[6];
+
+  language_names[0] = "auto";
+  language_names[1] = "local";
+  language_names[2] = "unknown";
+
+  const char **language_names_p = language_names;
+  /* language_names_p == &language_names[0].  */
+  language_names_p++;
+  /* language_names_p == &language_names[1].  */
+  language_names_p++;
+  /* language_names_p == &language_names[2].  */
+  language_names_p++;
+  /* language_names_p == &language_names[3].  */
+
+  const char **sort_begin;
+
+  if (0)
+    sort_begin = &language_names[3];
+  else
+    sort_begin = language_names_p;
+
+  language_names[3] = "";
+  language_names[4] = "";
+  language_names[5] = NULL;
+
+  /* There should be no warning associated with this std::sort as
+     sort_begin != &language_names[5] and GCC should be able to figure
+     that out.  */
+  std::sort (sort_begin, &language_names[5], compare_cstrings);
+}