[PATCH] include MEM_REF type in tree dumps (PR 90676)

[PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

I spent a bunch of time the other day trying to understand why
the second of the two assignments below to a char array was
apparently not being done by trunk

   a[0] = 1;
   a[1] = 0;

The optimized GIMPLE dump simply shows:

   MEM[(char *)&a] = 1;

when in the past it showed:

   MEM[(char[2] *)&a2] = 1;

After some debugging I figured out that this is the result of
the store merging pass transforming the two assignments into
one:

   *(short int *)a = 1;

and the MEM_REF dump mentioning only the type of the second
operand and not the type of the access.

To avoid this confusion the attached patch adds to the dump
a cast to the MEM_REF type for accesses whose size is not equal
to the size of the operand (when the sizes are the same no new
cast is prepended).  The effect is that with store merging in
effect, the dump for the above becomes

   MEM[(short int *)(char *)&a] = 1;

This should make both the size and the type of the access clear
and help avoid the confusion.  The output isn't the same as in
earlier releases because because the access really is done via
a short pointer and not as an array of char.

There is more detail in MEM_REF that could be included here but
it seems that the size of the access is essential to interpreting
the dumps.

Tested on x86_64-linux with only minimal testsuite fallout.

Martin

[-- Attachment #2: gcc-90676.diff --]
[-- Type: text/x-patch, Size: 8293 bytes --]

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
index cbe06b4ce62..08e0dd13115 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
@@ -11,7 +11,8 @@ void foo(void)
 	z = 1 + &a[1];
 }
 
-/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "&MEM\\\[\\(int \\*\\)\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { store_merge } } } } */
 
 
 void bar(int i)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
index 1fd8dec99e9..da9fc3b9c6a 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
@@ -97,5 +97,5 @@ int main()
 }
 
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
-
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[176] \\*\\)\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
new file mode 100644
index 00000000000..8b4a51c6cbf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
@@ -0,0 +1,37 @@
+/* PR middle-end/90676 - default GIMPLE dumps lack information
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-store-merging" }
+   { dg-require-effective-target int32plus }
+   { dg-require-effective-target store_merge } */
+
+
+extern char a2[2];
+
+void f2 (void)
+{
+  a2[0] = 1;
+  a2[1] = 0;
+}
+
+extern char a4[4];
+
+void f4 (void)
+{
+  a4[0] = 1;
+  a4[1] = 0;
+  a4[2] = 0;
+  a4[3] = 0;
+}
+
+extern char a8[8];
+
+void f8 (void)
+{
+  a8[0] = 1;
+  for (int i = 1; i != 8; ++i)
+    a8[i] = 0;
+}
+
+/* { dg-final { scan-tree-dump "MEM\\\[\\(unsigned short \\*\\)\\(char \\*\\)\\&a2] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM\\\[\\(unsigned int \\*\\)\\(char \\*\\)\\&a4] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM\\\[\\(unsigned long \\*\\)\\(char \\*\\)\\&a8] = " "store-merging"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
index 4494a2b0bd6..1d47e4c39eb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
@@ -22,5 +22,6 @@ void test_signed_msg_encoding(void)
     f();
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[8] \\*\\)\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
index 35b3d00ee44..ea9aad9c4cc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
@@ -19,4 +19,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+;" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
index 732d2324db5..37526a3d9fb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
@@ -23,4 +23,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
index a22cc7906da..8499f2bd28c 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
@@ -25,4 +25,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
index 282194c1e32..709adce32bf 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
@@ -59,4 +59,5 @@ void foo(int prec,
     bar (&info);
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[4] \\*\\)\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 4ba9170ddd3..649c81e06cd 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1690,21 +1690,32 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
 	  }
 	else
 	  {
-	    tree ptype;
-
 	    pp_string (pp, "MEM[");
+	    tree nodetype = TREE_TYPE (node);
+	    tree op0 = TREE_OPERAND (node, 0);
+	    tree op1 = TREE_OPERAND (node, 1);
+	    tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
+
+	    if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
+				     TYPE_SIZE (TREE_TYPE (op1type))))
+	      {
+		/* If the size of the type of the operand is not the same
+		   as the size of the MEM_REF expression include a cast
+		   to a pointer to the type of the latter to make it clear
+		   how many bytes of memory are being accessed.  */
+		pp_left_paren (pp);
+		dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
+		pp_string (pp, " *)");
+	      }
+
 	    pp_left_paren (pp);
-	    ptype = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1)));
-	    dump_generic_node (pp, ptype,
-			       spc, flags | TDF_SLIM, false);
+	    dump_generic_node (pp, op1type, spc, flags | TDF_SLIM, false);
 	    pp_right_paren (pp);
-	    dump_generic_node (pp, TREE_OPERAND (node, 0),
-			       spc, flags, false);
-	    if (!integer_zerop (TREE_OPERAND (node, 1)))
+	    dump_generic_node (pp, op0, spc, flags, false);
+	    if (!integer_zerop (op1))
 	      {
 		pp_string (pp, " + ");
-		dump_generic_node (pp, TREE_OPERAND (node, 1),
-				   spc, flags, false);
+		dump_generic_node (pp, op1, spc, flags, false);
 	      }
 	    if ((flags & TDF_ALIAS)
 		&& MR_DEPENDENCE_CLIQUE (node) != 0)

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]

A patch with a ChangeLog this time is attached.

On 6/1/19 9:53 AM, Martin Sebor wrote:
> I spent a bunch of time the other day trying to understand why
> the second of the two assignments below to a char array was
> apparently not being done by trunk
> 
>    a[0] = 1;
>    a[1] = 0;
> 
> The optimized GIMPLE dump simply shows:
> 
>    MEM[(char *)&a] = 1;
> 
> when in the past it showed:
> 
>    MEM[(char[2] *)&a2] = 1;
> 
> After some debugging I figured out that this is the result of
> the store merging pass transforming the two assignments into
> one:
> 
>    *(short int *)a = 1;
> 
> and the MEM_REF dump mentioning only the type of the second
> operand and not the type of the access.
> 
> To avoid this confusion the attached patch adds to the dump
> a cast to the MEM_REF type for accesses whose size is not equal
> to the size of the operand (when the sizes are the same no new
> cast is prepended).  The effect is that with store merging in
> effect, the dump for the above becomes
> 
>    MEM[(short int *)(char *)&a] = 1;
> 
> This should make both the size and the type of the access clear
> and help avoid the confusion.  The output isn't the same as in
> earlier releases because because the access really is done via
> a short pointer and not as an array of char.
> 
> There is more detail in MEM_REF that could be included here but
> it seems that the size of the access is essential to interpreting
> the dumps.
> 
> Tested on x86_64-linux with only minimal testsuite fallout.
> 
> Martin


[-- Attachment #2: gcc-90676.diff --]
[-- Type: text/x-patch, Size: 8876 bytes --]

PR middle-end/90676 - default GIMPLE dumps lack information

gcc/ChangeLog:

	PR middle-end/90676
	* tree-pretty-print.c (dump_generic_node): Include MEM_REF type
	in output when different size than operand.

gcc/testsuite/ChangeLog:

	PR middle-end/90676
	* gcc.dg/tree-ssa/dump-6.c: New test.
	* g++.dg/tree-ssa/pr19807.C: Adjust expected output.
	* g++.dg/tree-ssa/ssa-dse-1.C: Same.
	* gcc.dg/tree-ssa/pr30375.c: Same.
	* gcc.dg/tree-ssa/slsr-27.c (f): Same.
	* gcc.dg/tree-ssa/slsr-28.c (f): Same.
	* gcc.dg/tree-ssa/slsr-29.c (f): Same.
	* gcc.dg/tree-ssa/ssa-dse-24.c: Same.

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
index cbe06b4ce62..08e0dd13115 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
@@ -11,7 +11,8 @@ void foo(void)
 	z = 1 + &a[1];
 }
 
-/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "&MEM\\\[\\(int \\*\\)\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { store_merge } } } } */
 
 
 void bar(int i)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
index 1fd8dec99e9..da9fc3b9c6a 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
@@ -97,5 +97,5 @@ int main()
 }
 
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
-
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[176] \\*\\)\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
new file mode 100644
index 00000000000..8b4a51c6cbf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
@@ -0,0 +1,37 @@
+/* PR middle-end/90676 - default GIMPLE dumps lack information
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-store-merging" }
+   { dg-require-effective-target int32plus }
+   { dg-require-effective-target store_merge } */
+
+
+extern char a2[2];
+
+void f2 (void)
+{
+  a2[0] = 1;
+  a2[1] = 0;
+}
+
+extern char a4[4];
+
+void f4 (void)
+{
+  a4[0] = 1;
+  a4[1] = 0;
+  a4[2] = 0;
+  a4[3] = 0;
+}
+
+extern char a8[8];
+
+void f8 (void)
+{
+  a8[0] = 1;
+  for (int i = 1; i != 8; ++i)
+    a8[i] = 0;
+}
+
+/* { dg-final { scan-tree-dump "MEM\\\[\\(unsigned short \\*\\)\\(char \\*\\)\\&a2] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM\\\[\\(unsigned int \\*\\)\\(char \\*\\)\\&a4] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM\\\[\\(unsigned long \\*\\)\\(char \\*\\)\\&a8] = " "store-merging"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
index 4494a2b0bd6..1d47e4c39eb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
@@ -22,5 +22,6 @@ void test_signed_msg_encoding(void)
     f();
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[8] \\*\\)\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
index 35b3d00ee44..ea9aad9c4cc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
@@ -19,4 +19,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+;" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
index 732d2324db5..37526a3d9fb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
@@ -23,4 +23,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
index a22cc7906da..8499f2bd28c 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
@@ -25,4 +25,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(int \\*\\)\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
index 282194c1e32..709adce32bf 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
@@ -59,4 +59,5 @@ void foo(int prec,
     bar (&info);
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM\\\[\\(char\\\[4] \\*\\)\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 4ba9170ddd3..649c81e06cd 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1690,21 +1690,32 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
 	  }
 	else
 	  {
-	    tree ptype;
-
 	    pp_string (pp, "MEM[");
+	    tree nodetype = TREE_TYPE (node);
+	    tree op0 = TREE_OPERAND (node, 0);
+	    tree op1 = TREE_OPERAND (node, 1);
+	    tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
+
+	    if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
+				     TYPE_SIZE (TREE_TYPE (op1type))))
+	      {
+		/* If the size of the type of the operand is not the same
+		   as the size of the MEM_REF expression include a cast
+		   to a pointer to the type of the latter to make it clear
+		   how many bytes of memory are being accessed.  */
+		pp_left_paren (pp);
+		dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
+		pp_string (pp, " *)");
+	      }
+
 	    pp_left_paren (pp);
-	    ptype = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1)));
-	    dump_generic_node (pp, ptype,
-			       spc, flags | TDF_SLIM, false);
+	    dump_generic_node (pp, op1type, spc, flags | TDF_SLIM, false);
 	    pp_right_paren (pp);
-	    dump_generic_node (pp, TREE_OPERAND (node, 0),
-			       spc, flags, false);
-	    if (!integer_zerop (TREE_OPERAND (node, 1)))
+	    dump_generic_node (pp, op0, spc, flags, false);
+	    if (!integer_zerop (op1))
 	      {
 		pp_string (pp, " + ");
-		dump_generic_node (pp, TREE_OPERAND (node, 1),
-				   spc, flags, false);
+		dump_generic_node (pp, op1, spc, flags, false);
 	      }
 	    if ((flags & TDF_ALIAS)
 		&& MR_DEPENDENCE_CLIQUE (node) != 0)

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Sat, Jun 1, 2019 at 5:53 PM Martin Sebor <msebor@gmail.com> wrote:
>
> I spent a bunch of time the other day trying to understand why
> the second of the two assignments below to a char array was
> apparently not being done by trunk
>
>    a[0] = 1;
>    a[1] = 0;
>
> The optimized GIMPLE dump simply shows:
>
>    MEM[(char *)&a] = 1;
>
> when in the past it showed:
>
>    MEM[(char[2] *)&a2] = 1;
>
> After some debugging I figured out that this is the result of
> the store merging pass transforming the two assignments into
> one:
>
>    *(short int *)a = 1;
>
> and the MEM_REF dump mentioning only the type of the second
> operand and not the type of the access.
>
> To avoid this confusion the attached patch adds to the dump
> a cast to the MEM_REF type for accesses whose size is not equal
> to the size of the operand (when the sizes are the same no new
> cast is prepended).  The effect is that with store merging in
> effect, the dump for the above becomes
>
>    MEM[(short int *)(char *)&a] = 1;

I think this is confusing syntax.  Iff you absolutely refuse to
make the -gimple dump the default for MEM_REF and you insist
on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
which is the only other tree code we dump the access type, thus

 MEM<short int *>[(char *)&a] = 1;

btw, -gimple (iff only applied to MEM_REF) would have dumped

 __MEM <short int, 8> ((char *)&a2) = 1;

also including the effective alignment of the access.  Your variant
suggests that the alignment is that of short int which is wrong.

Yes, there's testsuite fallout I guess mostly about () vs. [] and spacing
(the GIMPLE FE doesn't insist on spacing before/after <>).

So please do not change MEM_REF dumping in this way.

Richard.

> This should make both the size and the type of the access clear
> and help avoid the confusion.  The output isn't the same as in
> earlier releases because because the access really is done via
> a short pointer and not as an array of char.
>
> There is more detail in MEM_REF that could be included here but
> it seems that the size of the access is essential to interpreting
> the dumps.
>
> Tested on x86_64-linux with only minimal testsuite fallout.
>
> Martin

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jakub Jelinek]

On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
> > To avoid this confusion the attached patch adds to the dump
> > a cast to the MEM_REF type for accesses whose size is not equal
> > to the size of the operand (when the sizes are the same no new
> > cast is prepended).  The effect is that with store merging in
> > effect, the dump for the above becomes
> >
> >    MEM[(short int *)(char *)&a] = 1;
> 
> I think this is confusing syntax.  Iff you absolutely refuse to
> make the -gimple dump the default for MEM_REF and you insist
> on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
> which is the only other tree code we dump the access type, thus

I must say I prefer the current MEM[ over the -gimple for human readable
dumps.

>  MEM<short int *>[(char *)&a] = 1;

Wouldn't that be
  MEM<short int>[(char *)&a] instead?
Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
is not compatible with TREE_TYPE (mem), so keep what we were doing in most
cases?

	Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
> > > To avoid this confusion the attached patch adds to the dump
> > > a cast to the MEM_REF type for accesses whose size is not equal
> > > to the size of the operand (when the sizes are the same no new
> > > cast is prepended).  The effect is that with store merging in
> > > effect, the dump for the above becomes
> > >
> > >    MEM[(short int *)(char *)&a] = 1;
> >
> > I think this is confusing syntax.  Iff you absolutely refuse to
> > make the -gimple dump the default for MEM_REF and you insist
> > on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
> > which is the only other tree code we dump the access type, thus
>
> I must say I prefer the current MEM[ over the -gimple for human readable
> dumps.

Sure, but then why ask for all information to be present when in the cases
you are curious you can look at -gimple dumps?  A similar thing I've
hacked the pretty printer locally for debugging in the past is alignment info.

> >  MEM<short int *>[(char *)&a] = 1;
>
> Wouldn't that be
>   MEM<short int>[(char *)&a] instead?

Err, yes.

> Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
> is not compatible with TREE_TYPE (mem), so keep what we were doing in most
> cases?

We could.  Like we dump MEM_REF as * in some cases.

The question is still why fix things half-way if a complete solution
is already there?

Btw, VIEW_CONVERT dumping uses () instead of [], that I used
[] when I introduced MEM_REF was probably a mistake...
Is it just the parens kind you dislike?

Richard.

>
>         Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

On 6/3/19 4:34 AM, Richard Biener wrote:
> On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
>>>> To avoid this confusion the attached patch adds to the dump
>>>> a cast to the MEM_REF type for accesses whose size is not equal
>>>> to the size of the operand (when the sizes are the same no new
>>>> cast is prepended).  The effect is that with store merging in
>>>> effect, the dump for the above becomes
>>>>
>>>>     MEM[(short int *)(char *)&a] = 1;
>>>
>>> I think this is confusing syntax.  Iff you absolutely refuse to
>>> make the -gimple dump the default for MEM_REF and you insist
>>> on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
>>> which is the only other tree code we dump the access type, thus
>>
>> I must say I prefer the current MEM[ over the -gimple for human readable
>> dumps.
> 
> Sure, but then why ask for all information to be present when in the cases
> you are curious you can look at -gimple dumps?  A similar thing I've
> hacked the pretty printer locally for debugging in the past is alignment info.
> 
>>>   MEM<short int *>[(char *)&a] = 1;
>>
>> Wouldn't that be
>>    MEM<short int>[(char *)&a] instead?
> 
> Err, yes.
> 
>> Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
>> is not compatible with TREE_TYPE (mem), so keep what we were doing in most
>> cases?
> 
> We could.  Like we dump MEM_REF as * in some cases.
> 
> The question is still why fix things half-way if a complete solution
> is already there?

Because it restores the important detail for those of us who
are accustomed to the "legacy" format.  That's without a doubt
the majority of users.  Note that godbolt.org only exposes
the classic dumps and doesn't make it possible to select
the -gimple form.

Those with a preference for the -gimple syntax are presumably
already using the -gimple dumps so they shouldn't be bothered
by a change to the legacy format.

But those with a preference for the traditional syntax will
not appreciate having the syntax changed.  Scripts that parse
those dumps (like GCC's own test harness) have a reasonable
chance of continuing to be able to parse the syntax even with
the additional cast.  They will certainly not be able to parse
it if it changes to MEM<T>(...)).

But if you refuse to accept the patch as is and insist on
the syntax with the pointy brackets please let me know.  I
think it's more important get the size of the access restored
than the details of the syntax so I'm willing to spend the time
to adjust the fix, even at the risk of breaking scripts and
making some users unhappy.

Martin

> 
> Btw, VIEW_CONVERT dumping uses () instead of [], that I used
> [] when I introduced MEM_REF was probably a mistake...
> Is it just the parens kind you dislike?
> 
> Richard.
> 
>>
>>          Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Mon, Jun 3, 2019 at 5:13 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 6/3/19 4:34 AM, Richard Biener wrote:
> > On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >>
> >> On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
> >>>> To avoid this confusion the attached patch adds to the dump
> >>>> a cast to the MEM_REF type for accesses whose size is not equal
> >>>> to the size of the operand (when the sizes are the same no new
> >>>> cast is prepended).  The effect is that with store merging in
> >>>> effect, the dump for the above becomes
> >>>>
> >>>>     MEM[(short int *)(char *)&a] = 1;
> >>>
> >>> I think this is confusing syntax.  Iff you absolutely refuse to
> >>> make the -gimple dump the default for MEM_REF and you insist
> >>> on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
> >>> which is the only other tree code we dump the access type, thus
> >>
> >> I must say I prefer the current MEM[ over the -gimple for human readable
> >> dumps.
> >
> > Sure, but then why ask for all information to be present when in the cases
> > you are curious you can look at -gimple dumps?  A similar thing I've
> > hacked the pretty printer locally for debugging in the past is alignment info.
> >
> >>>   MEM<short int *>[(char *)&a] = 1;
> >>
> >> Wouldn't that be
> >>    MEM<short int>[(char *)&a] instead?
> >
> > Err, yes.
> >
> >> Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
> >> is not compatible with TREE_TYPE (mem), so keep what we were doing in most
> >> cases?
> >
> > We could.  Like we dump MEM_REF as * in some cases.
> >
> > The question is still why fix things half-way if a complete solution
> > is already there?
>
> Because it restores the important detail for those of us who
> are accustomed to the "legacy" format.  That's without a doubt
> the majority of users.  Note that godbolt.org only exposes
> the classic dumps and doesn't make it possible to select
> the -gimple form.
>
> Those with a preference for the -gimple syntax are presumably
> already using the -gimple dumps so they shouldn't be bothered
> by a change to the legacy format.
>
> But those with a preference for the traditional syntax will
> not appreciate having the syntax changed.  Scripts that parse
> those dumps (like GCC's own test harness) have a reasonable
> chance of continuing to be able to parse the syntax even with
> the additional cast.  They will certainly not be able to parse
> it if it changes to MEM<T>(...)).
>
> But if you refuse to accept the patch as is and insist on
> the syntax with the pointy brackets please let me know.  I
> think it's more important get the size of the access restored
> than the details of the syntax so I'm willing to spend the time
> to adjust the fix, even at the risk of breaking scripts and
> making some users unhappy.

I think introducing inconsistencies (and I find two "casts"
confusing as well) with existing VIEW_CONVERT_EXPR
dumping isn't good.  So yes, I'd rather prefer

MEM <access-type> [(alias-pointer-type) ptr]

note access-type isn't a pointer type to the access type.

I can definitely live with this incremental but consistent change.
Also consider eliding access-type dumping as Jakub suggested
(when equal to *alias-pointer-type).

As said in the PR dump format changes have the chance
to make testcases testing for sth _not_ to appear no longer
testing what they want to test for (one reason those testcases
are broken).  A quick grep for MEM on a scan-*-dump-not
might reveal candidates that could need a second look.

Note that it was pure laziness on my side that I didn't change
the "legacy" format for the way the GIMPLE FE likes it :/
And I feel sorry about that.

Richard.

> Martin
>
> >
> > Btw, VIEW_CONVERT dumping uses () instead of [], that I used
> > [] when I introduced MEM_REF was probably a mistake...
> > Is it just the parens kind you dislike?
> >
> > Richard.
> >
> >>
> >>          Jakub
>

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]

The attached update adds the MEM_REF type in pointy braces like
in the -gimple format, but without the access size:

   MEM <short int> [(char *)&a] = 1;

Martin

On 6/4/19 4:57 AM, Richard Biener wrote:
> On Mon, Jun 3, 2019 at 5:13 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 6/3/19 4:34 AM, Richard Biener wrote:
>>> On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>>>
>>>> On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
>>>>>> To avoid this confusion the attached patch adds to the dump
>>>>>> a cast to the MEM_REF type for accesses whose size is not equal
>>>>>> to the size of the operand (when the sizes are the same no new
>>>>>> cast is prepended).  The effect is that with store merging in
>>>>>> effect, the dump for the above becomes
>>>>>>
>>>>>>      MEM[(short int *)(char *)&a] = 1;
>>>>>
>>>>> I think this is confusing syntax.  Iff you absolutely refuse to
>>>>> make the -gimple dump the default for MEM_REF and you insist
>>>>> on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
>>>>> which is the only other tree code we dump the access type, thus
>>>>
>>>> I must say I prefer the current MEM[ over the -gimple for human readable
>>>> dumps.
>>>
>>> Sure, but then why ask for all information to be present when in the cases
>>> you are curious you can look at -gimple dumps?  A similar thing I've
>>> hacked the pretty printer locally for debugging in the past is alignment info.
>>>
>>>>>    MEM<short int *>[(char *)&a] = 1;
>>>>
>>>> Wouldn't that be
>>>>     MEM<short int>[(char *)&a] instead?
>>>
>>> Err, yes.
>>>
>>>> Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
>>>> is not compatible with TREE_TYPE (mem), so keep what we were doing in most
>>>> cases?
>>>
>>> We could.  Like we dump MEM_REF as * in some cases.
>>>
>>> The question is still why fix things half-way if a complete solution
>>> is already there?
>>
>> Because it restores the important detail for those of us who
>> are accustomed to the "legacy" format.  That's without a doubt
>> the majority of users.  Note that godbolt.org only exposes
>> the classic dumps and doesn't make it possible to select
>> the -gimple form.
>>
>> Those with a preference for the -gimple syntax are presumably
>> already using the -gimple dumps so they shouldn't be bothered
>> by a change to the legacy format.
>>
>> But those with a preference for the traditional syntax will
>> not appreciate having the syntax changed.  Scripts that parse
>> those dumps (like GCC's own test harness) have a reasonable
>> chance of continuing to be able to parse the syntax even with
>> the additional cast.  They will certainly not be able to parse
>> it if it changes to MEM<T>(...)).
>>
>> But if you refuse to accept the patch as is and insist on
>> the syntax with the pointy brackets please let me know.  I
>> think it's more important get the size of the access restored
>> than the details of the syntax so I'm willing to spend the time
>> to adjust the fix, even at the risk of breaking scripts and
>> making some users unhappy.
> 
> I think introducing inconsistencies (and I find two "casts"
> confusing as well) with existing VIEW_CONVERT_EXPR
> dumping isn't good.  So yes, I'd rather prefer
> 
> MEM <access-type> [(alias-pointer-type) ptr]
> 
> note access-type isn't a pointer type to the access type.
> 
> I can definitely live with this incremental but consistent change.
> Also consider eliding access-type dumping as Jakub suggested
> (when equal to *alias-pointer-type).
> 
> As said in the PR dump format changes have the chance
> to make testcases testing for sth _not_ to appear no longer
> testing what they want to test for (one reason those testcases
> are broken).  A quick grep for MEM on a scan-*-dump-not
> might reveal candidates that could need a second look.
> 
> Note that it was pure laziness on my side that I didn't change
> the "legacy" format for the way the GIMPLE FE likes it :/
> And I feel sorry about that.
> 
> Richard.
> 
>> Martin
>>
>>>
>>> Btw, VIEW_CONVERT dumping uses () instead of [], that I used
>>> [] when I introduced MEM_REF was probably a mistake...
>>> Is it just the parens kind you dislike?
>>>
>>> Richard.
>>>
>>>>
>>>>           Jakub
>>


[-- Attachment #2: gcc-90676.diff --]
[-- Type: text/x-patch, Size: 17356 bytes --]

PR middle-end/90676 - default GIMPLE dumps lack information

gcc/ChangeLog:

	PR middle-end/90676
	* tree-pretty-print.c (dump_mem_ref): New function.  Include
	MEM_REF type in output when different size than operand.
	(dump_generic_node): Move code to dump_mem_ref and call it.

gcc/testsuite/ChangeLog:

	PR middle-end/90676
	* gcc.dg/tree-ssa/dump-6.c: New test.
	* g++.dg/tree-ssa/pr19807.C: Adjust expected output.
	* g++.dg/tree-ssa/ssa-dse-1.C: Same.
	* gcc.dg/tree-prof/stringop-2.c
	* gcc.dg/tree-ssa/pr30375.c: Same.
	* gcc.dg/tree-ssa/slsr-27.c (f): Same.
	* gcc.dg/tree-ssa/slsr-28.c (f): Same.
	* gcc.dg/tree-ssa/slsr-29.c (f): Same.
	* gcc.dg/tree-ssa/ssa-dse-24.c: Same.

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
index cbe06b4ce62..10de295e14d 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr19807.C
@@ -11,7 +11,8 @@ void foo(void)
 	z = 1 + &a[1];
 }
 
-/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "&MEM\\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "&MEM <int> \\\[\\\(void .\\\)&a \\\+ 8B\\\]" 3 "optimized" { target { store_merge } } } } */
 
 
 void bar(int i)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
index 1fd8dec99e9..7bcb65af13c 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C
@@ -97,5 +97,5 @@ int main()
 }
 
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
-
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM <char\\\[176]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/store_merging_5.c b/gcc/testsuite/gcc.dg/store_merging_5.c
index e6c349767bc..e46d6e5563a 100644
--- a/gcc/testsuite/gcc.dg/store_merging_5.c
+++ b/gcc/testsuite/gcc.dg/store_merging_5.c
@@ -26,5 +26,6 @@ foo1 (struct bar *p, char tmp)
 }
 
 
-/* { dg-final { scan-tree-dump-times "Merging successful" 1 "store-merging" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[.*\\\]" 1 "store-merging" } } */
+/* { dg-final { scan-tree-dump-times "Merging successful" 1 "store-merging" } }
+   { dg-final { scan-tree-dump-times "MEM\\\[.*\\\]" 1 "store-merging" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM <unsigned long>\\\[.*\\\]" 1 "store-merging" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-prof/stringop-2.c b/gcc/testsuite/gcc.dg/tree-prof/stringop-2.c
index 3242cf5b8a2..c1f757388fd 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/stringop-2.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/stringop-2.c
@@ -21,5 +21,6 @@ main()
 }
 /* autofdo doesn't support value profiling for now: */
 /* { dg-final-use-not-autofdo { scan-ipa-dump "Transformation done: single value 4 stringop" "profile"} } */
-/* The versioned memset of size 4 should be optimized to an assignment.  */
-/* { dg-final-use-not-autofdo { scan-tree-dump "MEM\\\[\\(void .\\)&a\\\] = 168430090" "optimized"} } */
+/* The versioned memset of size 4 should be optimized to an assignment.
+   { dg-final-use-not-autofdo { scan-tree-dump "MEM\\\[\\(void .\\)&a\\\] = 168430090" "optimized" { target { ! store_merge } } } }
+   { dg-final-use-not-autofdo { scan-tree-dump "MEM <\[a-z \]+> \\\[\\(void .\\)&a\\\] = 168430090" "optimized" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
new file mode 100644
index 00000000000..b66d3486e88
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
@@ -0,0 +1,37 @@
+/* PR middle-end/90676 - default GIMPLE dumps lack information
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-store-merging" }
+   { dg-require-effective-target int32plus }
+   { dg-require-effective-target store_merge } */
+
+
+extern char a2[2];
+
+void f2 (void)
+{
+  a2[0] = 1;
+  a2[1] = 0;
+}
+
+extern char a4[4];
+
+void f4 (void)
+{
+  a4[0] = 1;
+  a4[1] = 0;
+  a4[2] = 0;
+  a4[3] = 0;
+}
+
+extern char a8[8];
+
+void f8 (void)
+{
+  a8[0] = 1;
+  for (int i = 1; i != 8; ++i)
+    a8[i] = 0;
+}
+
+/* { dg-final { scan-tree-dump "MEM <unsigned short> \\\[\\(char \\*\\)\\&a2] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM <unsigned int> \\\[\\(char \\*\\)\\&a4] = " "store-merging"} }
+   { dg-final { scan-tree-dump "MEM <unsigned long> \\\[\\(char \\*\\)\\&a8] = " "store-merging"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
index 4494a2b0bd6..5c068a3ddc0 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr30375.c
@@ -22,5 +22,6 @@ void test_signed_msg_encoding(void)
     f();
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM <char\\\[8]> \\\[\\(struct _s \\*\\)&signInfo \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
index 35b3d00ee44..e3560ee5d9a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-27.c
@@ -19,4 +19,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+;" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM <int> \\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 3 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
index 732d2324db5..0db27af76ce 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-28.c
@@ -23,4 +23,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM <int> \\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
index a22cc7906da..0f6169df312 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/slsr-29.c
@@ -25,4 +25,5 @@ f (struct x *p, unsigned int n)
 /* { dg-final { scan-tree-dump-times "\\* 4;" 1 "dom3" { target { int32 } } } } */
 /* { dg-final { scan-tree-dump-times "\\* 2;" 1 "dom3" { target { int16 } } } } */
 /* { dg-final { scan-tree-dump-times "p_\\d\+\\(D\\) \\+ \[^\r\n\]*_\\d\+" 1 "dom3" } } */
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { ! store_merge } } } } */
+/* { dg-final { scan-tree-dump-times "MEM <int> \\\[\\(struct x \\*\\)\[^\r\n\]*_\\d\+" 9 "dom3" { target { store_merge } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
index 282194c1e32..240c9063717 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
@@ -59,4 +59,5 @@ void foo(int prec,
     bar (&info);
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM <char[4]> \\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 1e6c96813bb..41dd6db53a9 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1378,6 +1378,126 @@ dump_omp_atomic_memory_order (pretty_printer *pp, enum omp_memory_order mo)
     }
 }
 
+/* Helper to dump a MEM_REF node.  */
+
+static void
+dump_mem_ref (pretty_printer *pp, tree node, int spc, dump_flags_t flags)
+{
+  if (flags & TDF_GIMPLE)
+    {
+      pp_string (pp, "__MEM <");
+      dump_generic_node (pp, TREE_TYPE (node),
+			 spc, flags | TDF_SLIM, false);
+      if (TYPE_ALIGN (TREE_TYPE (node))
+	  != TYPE_ALIGN (TYPE_MAIN_VARIANT (TREE_TYPE (node))))
+	{
+	  pp_string (pp, ", ");
+	  pp_decimal_int (pp, TYPE_ALIGN (TREE_TYPE (node)));
+	}
+      pp_greater (pp);
+      pp_string (pp, " (");
+      if (TREE_TYPE (TREE_OPERAND (node, 0))
+	  != TREE_TYPE (TREE_OPERAND (node, 1)))
+	{
+	  pp_left_paren (pp);
+	  dump_generic_node (pp, TREE_TYPE (TREE_OPERAND (node, 1)),
+			     spc, flags | TDF_SLIM, false);
+	  pp_right_paren (pp);
+	}
+      dump_generic_node (pp, TREE_OPERAND (node, 0),
+			 spc, flags | TDF_SLIM, false);
+      if (! integer_zerop (TREE_OPERAND (node, 1)))
+	{
+	  pp_string (pp, " + ");
+	  dump_generic_node (pp, TREE_OPERAND (node, 1),
+			     spc, flags | TDF_SLIM, false);
+	}
+      pp_right_paren (pp);
+    }
+  else if (integer_zerop (TREE_OPERAND (node, 1))
+	   /* Dump the types of INTEGER_CSTs explicitly, for we can't
+	      infer them and MEM_ATTR caching will share MEM_REFs
+	      with differently-typed op0s.  */
+	   && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
+	   /* Released SSA_NAMES have no TREE_TYPE.  */
+	   && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
+	   /* Same pointer types, but ignoring POINTER_TYPE vs.
+	      REFERENCE_TYPE.  */
+	   && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
+	       == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
+	   && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
+	       == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
+	   && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
+	       == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
+	   /* Same value types ignoring qualifiers.  */
+	   && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
+	       == TYPE_MAIN_VARIANT
+	       (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
+	   && (!(flags & TDF_ALIAS)
+	       || MR_DEPENDENCE_CLIQUE (node) == 0))
+    {
+      if (TREE_CODE (TREE_OPERAND (node, 0)) != ADDR_EXPR)
+	{
+	  /* Enclose pointers to arrays in parentheses.  */
+	  tree op0 = TREE_OPERAND (node, 0);
+	  tree op0type = TREE_TYPE (op0);
+	  if (POINTER_TYPE_P (op0type)
+	      && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+	    pp_left_paren (pp);
+	  pp_star (pp);
+	  dump_generic_node (pp, op0, spc, flags, false);
+	  if (POINTER_TYPE_P (op0type)
+	      && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+	    pp_right_paren (pp);
+	}
+      else
+	dump_generic_node (pp,
+			   TREE_OPERAND (TREE_OPERAND (node, 0), 0),
+			   spc, flags, false);
+    }
+  else
+    {
+      pp_string (pp, "MEM");
+
+      tree nodetype = TREE_TYPE (node);
+      tree op0 = TREE_OPERAND (node, 0);
+      tree op1 = TREE_OPERAND (node, 1);
+      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
+
+      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
+			       TYPE_SIZE (TREE_TYPE (op1type))))
+	{
+	  pp_string (pp, " <");
+	  /* If the size of the type of the operand is not the same
+	     as the size of the MEM_REF expression include the type
+	     of the latter similar to the TDF_GIMPLE output to make
+	     it clear how many bytes of memory are being accessed.  */
+	  dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
+	  pp_string (pp, "> ");
+	}
+
+      pp_string (pp, "[(");
+      dump_generic_node (pp, op1type, spc, flags | TDF_SLIM, false);
+      pp_right_paren (pp);
+      dump_generic_node (pp, op0, spc, flags, false);
+      if (!integer_zerop (op1))
+      if (!integer_zerop (TREE_OPERAND (node, 1)))
+	{
+	  pp_string (pp, " + ");
+	  dump_generic_node (pp, op1, spc, flags, false);
+	}
+      if ((flags & TDF_ALIAS)
+	  && MR_DEPENDENCE_CLIQUE (node) != 0)
+	{
+	  pp_string (pp, " clique ");
+	  pp_unsigned_wide_integer (pp, MR_DEPENDENCE_CLIQUE (node));
+	  pp_string (pp, " base ");
+	  pp_unsigned_wide_integer (pp, MR_DEPENDENCE_BASE (node));
+	}
+      pp_right_bracket (pp);
+    }
+ }
+
 /* Dump the node NODE on the pretty_printer PP, SPC spaces of
    indent.  FLAGS specifies details to show in the dump (see TDF_* in
    dumpfile.h).  If IS_STMT is true, the object printed is considered
@@ -1636,109 +1756,8 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
       break;
 
     case MEM_REF:
-      {
-	if (flags & TDF_GIMPLE)
-	  {
-	    pp_string (pp, "__MEM <");
-	    dump_generic_node (pp, TREE_TYPE (node),
-			       spc, flags | TDF_SLIM, false);
-	    if (TYPE_ALIGN (TREE_TYPE (node))
-		!= TYPE_ALIGN (TYPE_MAIN_VARIANT (TREE_TYPE (node))))
-	      {
-		pp_string (pp, ", ");
-		pp_decimal_int (pp, TYPE_ALIGN (TREE_TYPE (node)));
-	      }
-	    pp_greater (pp);
-	    pp_string (pp, " (");
-	    if (TREE_TYPE (TREE_OPERAND (node, 0))
-		!= TREE_TYPE (TREE_OPERAND (node, 1)))
-	      {
-		pp_left_paren (pp);
-		dump_generic_node (pp, TREE_TYPE (TREE_OPERAND (node, 1)),
-				   spc, flags | TDF_SLIM, false);
-		pp_right_paren (pp);
-	      }
-	    dump_generic_node (pp, TREE_OPERAND (node, 0),
-			       spc, flags | TDF_SLIM, false);
-	    if (! integer_zerop (TREE_OPERAND (node, 1)))
-	      {
-		pp_string (pp, " + ");
-		dump_generic_node (pp, TREE_OPERAND (node, 1),
-				   spc, flags | TDF_SLIM, false);
-	      }
-	    pp_right_paren (pp);
-	  }
-	else if (integer_zerop (TREE_OPERAND (node, 1))
-	    /* Dump the types of INTEGER_CSTs explicitly, for we can't
-	       infer them and MEM_ATTR caching will share MEM_REFs
-	       with differently-typed op0s.  */
-	    && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
-	    /* Released SSA_NAMES have no TREE_TYPE.  */
-	    && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
-	    /* Same pointer types, but ignoring POINTER_TYPE vs.
-	       REFERENCE_TYPE.  */
-	    && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
-		== TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
-	    && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
-		== TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
-	    && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
-		== TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
-	    /* Same value types ignoring qualifiers.  */
-	    && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
-		== TYPE_MAIN_VARIANT
-		    (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
-	    && (!(flags & TDF_ALIAS)
-		|| MR_DEPENDENCE_CLIQUE (node) == 0))
-	  {
-	    if (TREE_CODE (TREE_OPERAND (node, 0)) != ADDR_EXPR)
-	      {
-		/* Enclose pointers to arrays in parentheses.  */
-		tree op0 = TREE_OPERAND (node, 0);
-		tree op0type = TREE_TYPE (op0);
-		if (POINTER_TYPE_P (op0type)
-		    && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
-		  pp_left_paren (pp);
-		pp_star (pp);
-		dump_generic_node (pp, op0, spc, flags, false);
-		if (POINTER_TYPE_P (op0type)
-		    && TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
-		  pp_right_paren (pp);
-	      }
-	    else
-	      dump_generic_node (pp,
-				 TREE_OPERAND (TREE_OPERAND (node, 0), 0),
-				 spc, flags, false);
-	  }
-	else
-	  {
-	    tree ptype;
-
-	    pp_string (pp, "MEM[");
-	    pp_left_paren (pp);
-	    ptype = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (node, 1)));
-	    dump_generic_node (pp, ptype,
-			       spc, flags | TDF_SLIM, false);
-	    pp_right_paren (pp);
-	    dump_generic_node (pp, TREE_OPERAND (node, 0),
-			       spc, flags, false);
-	    if (!integer_zerop (TREE_OPERAND (node, 1)))
-	      {
-		pp_string (pp, " + ");
-		dump_generic_node (pp, TREE_OPERAND (node, 1),
-				   spc, flags, false);
-	      }
-	    if ((flags & TDF_ALIAS)
-		&& MR_DEPENDENCE_CLIQUE (node) != 0)
-	      {
-		pp_string (pp, " clique ");
-		pp_unsigned_wide_integer (pp, MR_DEPENDENCE_CLIQUE (node));
-		pp_string (pp, " base ");
-		pp_unsigned_wide_integer (pp, MR_DEPENDENCE_BASE (node));
-	      }
-	    pp_right_bracket (pp);
-	  }
-	break;
-      }
+      dump_mem_ref (pp, node, spc, flags);
+      break;
 
     case TARGET_MEM_REF:
       {

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jakub Jelinek]

On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
> +  else if (integer_zerop (TREE_OPERAND (node, 1))
> +	   /* Dump the types of INTEGER_CSTs explicitly, for we can't
> +	      infer them and MEM_ATTR caching will share MEM_REFs
> +	      with differently-typed op0s.  */
> +	   && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
> +	   /* Released SSA_NAMES have no TREE_TYPE.  */
> +	   && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
> +	   /* Same pointer types, but ignoring POINTER_TYPE vs.
> +	      REFERENCE_TYPE.  */
> +	   && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
> +	       == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
> +	   && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
> +	       == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
> +	   && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
> +	       == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
> +	   /* Same value types ignoring qualifiers.  */
> +	   && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
> +	       == TYPE_MAIN_VARIANT
> +	       (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))

You should be using types_compatible_p rather than type equality, that is
all GIMPLE cares about.

	Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

On 6/10/19 1:29 PM, Jakub Jelinek wrote:
> On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
>> +  else if (integer_zerop (TREE_OPERAND (node, 1))
>> +	   /* Dump the types of INTEGER_CSTs explicitly, for we can't
>> +	      infer them and MEM_ATTR caching will share MEM_REFs
>> +	      with differently-typed op0s.  */
>> +	   && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
>> +	   /* Released SSA_NAMES have no TREE_TYPE.  */
>> +	   && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
>> +	   /* Same pointer types, but ignoring POINTER_TYPE vs.
>> +	      REFERENCE_TYPE.  */
>> +	   && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
>> +	       == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
>> +	   && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
>> +	       == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
>> +	   && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
>> +	       == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
>> +	   /* Same value types ignoring qualifiers.  */
>> +	   && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
>> +	       == TYPE_MAIN_VARIANT
>> +	       (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
> 
> You should be using types_compatible_p rather than type equality, that is
> all GIMPLE cares about.

The code above predates my change, I just factored it out into
a separate function; it does make the diff bigger.  The code
I introduced only adds the <...> if the size of the access
differs from the size of the operand. I used type sizes rather
than testing their compatibility to minimize the number of tests
that might need updating.

This is the salient bit:

+      pp_string (pp, "MEM");
+
+      tree nodetype = TREE_TYPE (node);
+      tree op0 = TREE_OPERAND (node, 0);
+      tree op1 = TREE_OPERAND (node, 1);
+      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
+
+      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
+			       TYPE_SIZE (TREE_TYPE (op1type))))
+	{
+	  pp_string (pp, " <");
+	  /* If the size of the type of the operand is not the same
+	     as the size of the MEM_REF expression include the type
+	     of the latter similar to the TDF_GIMPLE output to make
+	     it clear how many bytes of memory are being accessed.  */
+	  dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
+	  pp_string (pp, "> ");
+	}

Martin

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Mon, Jun 10, 2019 at 10:37 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 6/10/19 1:29 PM, Jakub Jelinek wrote:
> > On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
> >> +  else if (integer_zerop (TREE_OPERAND (node, 1))
> >> +       /* Dump the types of INTEGER_CSTs explicitly, for we can't
> >> +          infer them and MEM_ATTR caching will share MEM_REFs
> >> +          with differently-typed op0s.  */
> >> +       && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
> >> +       /* Released SSA_NAMES have no TREE_TYPE.  */
> >> +       && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
> >> +       /* Same pointer types, but ignoring POINTER_TYPE vs.
> >> +          REFERENCE_TYPE.  */
> >> +       && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       /* Same value types ignoring qualifiers.  */
> >> +       && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
> >> +           == TYPE_MAIN_VARIANT
> >> +           (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
> >
> > You should be using types_compatible_p rather than type equality, that is
> > all GIMPLE cares about.
>
> The code above predates my change, I just factored it out into
> a separate function; it does make the diff bigger.  The code
> I introduced only adds the <...> if the size of the access
> differs from the size of the operand. I used type sizes rather
> than testing their compatibility to minimize the number of tests
> that might need updating.
>
> This is the salient bit:
>
> +      pp_string (pp, "MEM");
> +
> +      tree nodetype = TREE_TYPE (node);
> +      tree op0 = TREE_OPERAND (node, 0);
> +      tree op1 = TREE_OPERAND (node, 1);
> +      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
> +
> +      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
> +                              TYPE_SIZE (TREE_TYPE (op1type))))
> +       {
> +         pp_string (pp, " <");
> +         /* If the size of the type of the operand is not the same
> +            as the size of the MEM_REF expression include the type
> +            of the latter similar to the TDF_GIMPLE output to make
> +            it clear how many bytes of memory are being accessed.  */
> +         dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
> +         pp_string (pp, "> ");
> +       }

You need to guard against non-constant TYPE_SIZE here (for both
involved types) so I suggest you use operand_equal_p (..., 0).  If you
do that you need to guard against NULL_TREE TYPE_SIZE
(tree_int_cst_equal handled that as unequal).

OK with such change.
Richard.

> Martin

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Rainer Orth]

Richard Biener <richard.guenther@gmail.com> writes:

> On Mon, Jun 10, 2019 at 10:37 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 6/10/19 1:29 PM, Jakub Jelinek wrote:
>> > On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
>> >> +  else if (integer_zerop (TREE_OPERAND (node, 1))
>> >> +       /* Dump the types of INTEGER_CSTs explicitly, for we can't
>> >> +          infer them and MEM_ATTR caching will share MEM_REFs
>> >> +          with differently-typed op0s.  */
>> >> +       && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
>> >> +       /* Released SSA_NAMES have no TREE_TYPE.  */
>> >> +       && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
>> >> +       /* Same pointer types, but ignoring POINTER_TYPE vs.
>> >> +          REFERENCE_TYPE.  */
>> >> +       && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
>> >> +           == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
>> >> +       && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
>> >> +           == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
>> >> +       && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
>> >> +           == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
>> >> +       /* Same value types ignoring qualifiers.  */
>> >> +       && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
>> >> +           == TYPE_MAIN_VARIANT
>> >> +           (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
>> >
>> > You should be using types_compatible_p rather than type equality, that is
>> > all GIMPLE cares about.
>>
>> The code above predates my change, I just factored it out into
>> a separate function; it does make the diff bigger.  The code
>> I introduced only adds the <...> if the size of the access
>> differs from the size of the operand. I used type sizes rather
>> than testing their compatibility to minimize the number of tests
>> that might need updating.
>>
>> This is the salient bit:
>>
>> +      pp_string (pp, "MEM");
>> +
>> +      tree nodetype = TREE_TYPE (node);
>> +      tree op0 = TREE_OPERAND (node, 0);
>> +      tree op1 = TREE_OPERAND (node, 1);
>> +      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
>> +
>> +      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
>> +                              TYPE_SIZE (TREE_TYPE (op1type))))
>> +       {
>> +         pp_string (pp, " <");
>> +         /* If the size of the type of the operand is not the same
>> +            as the size of the MEM_REF expression include the type
>> +            of the latter similar to the TDF_GIMPLE output to make
>> +            it clear how many bytes of memory are being accessed.  */
>> +         dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
>> +         pp_string (pp, "> ");
>> +       }
>
> You need to guard against non-constant TYPE_SIZE here (for both
> involved types) so I suggest you use operand_equal_p (..., 0).  If you
> do that you need to guard against NULL_TREE TYPE_SIZE
> (tree_int_cst_equal handled that as unequal).
>
> OK with such change.
> Richard.

The testsuite part cannot have been tested very thoroughly: for one,
this snippet

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
@@ -59,4 +59,5 @@ void foo(int prec,
     bar (&info);
 }
 
-/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
+   { dg-final { scan-tree-dump-times "MEM <char[4]> \\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */


breaks gcc.sum creation with dg-extract-result.py (the default)
completely: gcc.log shows

spawn -ignore SIGHUP /var/gcc/regression/trunk/11-gcc/build/gcc/xgcc -B/var/gcc/regression/trunk/11-gcc/build/gcc/ /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O2 -fdump-tree-dse1 -S -o ssa-dse-24.s^M
PASS: gcc.dg/tree-ssa/ssa-dse-24.c (test for excess errors)
ERROR: (DejaGnu) proc "4" does not exist.
The error code is NONE
The info on the error is:
invalid command name "4"
    while executing
"::tcl_unknown 4"
    ("uplevel" body line 1)
    invoked from within
"uplevel 1 ::tcl_unknown $args"

due to the unquoted [4].

Even with that fixed, I see many failures:

+FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
+FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
+FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
+FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1

on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),

+FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++14  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
+FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++17  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
+FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++98  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3

+FAIL: gcc.dg/tree-prof/stringop-2.c scan-tree-dump optimized "MEM\\\\[\\\\(void .\\\\)&a\\\\] = 168430090"
+FAIL: gcc.dg/tree-ssa/pr30375.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct _s \\\\*\\\\)&signInfo \\\\+ [0-9]+B\\\\] = {}" 1
+FAIL: gcc.dg/tree-ssa/slsr-27.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 3
+FAIL: gcc.dg/tree-ssa/slsr-28.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
+FAIL: gcc.dg/tree-ssa/slsr-29.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
+FAIL: gcc.dg/tree-ssa/ssa-dse-24.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct printf_info \\\\*\\\\)&info \\\\+ [0-9]+B\\\\] = {}" 1

on 32 and 64-bit sparc-sun-solaris2.11 (the first of those also on
m68k-unknown-linux-gnu).

I haven't even started looking what's wrong (quoting problems in the REs
or other issues).

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

On 6/12/19 3:33 PM, Rainer Orth wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
> 
>> On Mon, Jun 10, 2019 at 10:37 PM Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> On 6/10/19 1:29 PM, Jakub Jelinek wrote:
>>>> On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
>>>>> +  else if (integer_zerop (TREE_OPERAND (node, 1))
>>>>> +       /* Dump the types of INTEGER_CSTs explicitly, for we can't
>>>>> +          infer them and MEM_ATTR caching will share MEM_REFs
>>>>> +          with differently-typed op0s.  */
>>>>> +       && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
>>>>> +       /* Released SSA_NAMES have no TREE_TYPE.  */
>>>>> +       && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
>>>>> +       /* Same pointer types, but ignoring POINTER_TYPE vs.
>>>>> +          REFERENCE_TYPE.  */
>>>>> +       && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
>>>>> +           == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
>>>>> +       && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
>>>>> +           == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
>>>>> +       && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
>>>>> +           == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
>>>>> +       /* Same value types ignoring qualifiers.  */
>>>>> +       && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
>>>>> +           == TYPE_MAIN_VARIANT
>>>>> +           (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
>>>>
>>>> You should be using types_compatible_p rather than type equality, that is
>>>> all GIMPLE cares about.
>>>
>>> The code above predates my change, I just factored it out into
>>> a separate function; it does make the diff bigger.  The code
>>> I introduced only adds the <...> if the size of the access
>>> differs from the size of the operand. I used type sizes rather
>>> than testing their compatibility to minimize the number of tests
>>> that might need updating.
>>>
>>> This is the salient bit:
>>>
>>> +      pp_string (pp, "MEM");
>>> +
>>> +      tree nodetype = TREE_TYPE (node);
>>> +      tree op0 = TREE_OPERAND (node, 0);
>>> +      tree op1 = TREE_OPERAND (node, 1);
>>> +      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
>>> +
>>> +      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
>>> +                              TYPE_SIZE (TREE_TYPE (op1type))))
>>> +       {
>>> +         pp_string (pp, " <");
>>> +         /* If the size of the type of the operand is not the same
>>> +            as the size of the MEM_REF expression include the type
>>> +            of the latter similar to the TDF_GIMPLE output to make
>>> +            it clear how many bytes of memory are being accessed.  */
>>> +         dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
>>> +         pp_string (pp, "> ");
>>> +       }
>>
>> You need to guard against non-constant TYPE_SIZE here (for both
>> involved types) so I suggest you use operand_equal_p (..., 0).  If you
>> do that you need to guard against NULL_TREE TYPE_SIZE
>> (tree_int_cst_equal handled that as unequal).
>>
>> OK with such change.
>> Richard.
> 
> The testsuite part cannot have been tested very thoroughly: for one,
> this snippet

Please try r272218.

Martin

> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> @@ -59,4 +59,5 @@ void foo(int prec,
>       bar (&info);
>   }
>   
> -/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
> +/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
> +   { dg-final { scan-tree-dump-times "MEM <char[4]> \\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> 
> 
> breaks gcc.sum creation with dg-extract-result.py (the default)
> completely: gcc.log shows
> 
> spawn -ignore SIGHUP /var/gcc/regression/trunk/11-gcc/build/gcc/xgcc -B/var/gcc/regression/trunk/11-gcc/build/gcc/ /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O2 -fdump-tree-dse1 -S -o ssa-dse-24.s^M
> PASS: gcc.dg/tree-ssa/ssa-dse-24.c (test for excess errors)
> ERROR: (DejaGnu) proc "4" does not exist.
> The error code is NONE
> The info on the error is:
> invalid command name "4"
>      while executing
> "::tcl_unknown 4"
>      ("uplevel" body line 1)
>      invoked from within
> "uplevel 1 ::tcl_unknown $args"
> 
> due to the unquoted [4].
> 
> Even with that fixed, I see many failures:
> 
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
> 
> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
> 
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++14  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++17  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++98  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> 
> +FAIL: gcc.dg/tree-prof/stringop-2.c scan-tree-dump optimized "MEM\\\\[\\\\(void .\\\\)&a\\\\] = 168430090"
> +FAIL: gcc.dg/tree-ssa/pr30375.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct _s \\\\*\\\\)&signInfo \\\\+ [0-9]+B\\\\] = {}" 1
> +FAIL: gcc.dg/tree-ssa/slsr-27.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 3
> +FAIL: gcc.dg/tree-ssa/slsr-28.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
> +FAIL: gcc.dg/tree-ssa/slsr-29.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
> +FAIL: gcc.dg/tree-ssa/ssa-dse-24.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct printf_info \\\\*\\\\)&info \\\\+ [0-9]+B\\\\] = {}" 1
> 
> on 32 and 64-bit sparc-sun-solaris2.11 (the first of those also on
> m68k-unknown-linux-gnu).
> 
> I haven't even started looking what's wrong (quoting problems in the REs
> or other issues).

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Rainer Orth]

Hi Martin,

>> The testsuite part cannot have been tested very thoroughly: for one,
>> this snippet
>
> Please try r272218.

this cured a couple of issues, like this one:

>> ERROR: (DejaGnu) proc "4" does not exist.

However...

>> Even with that fixed, I see many failures:
>>
>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
>>
>> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),

these failures remain...

>> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++14  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
>> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++17  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
>> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++98  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3

as does this one...

>> +FAIL: gcc.dg/tree-prof/stringop-2.c scan-tree-dump optimized "MEM\\\\[\\\\(void .\\\\)&a\\\\] = 168430090"

and this one...

>> +FAIL: gcc.dg/tree-ssa/pr30375.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct _s \\\\*\\\\)&signInfo \\\\+ [0-9]+B\\\\] = {}" 1
>> +FAIL: gcc.dg/tree-ssa/slsr-27.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 3
>> +FAIL: gcc.dg/tree-ssa/slsr-28.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
>> +FAIL: gcc.dg/tree-ssa/slsr-29.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
>> +FAIL: gcc.dg/tree-ssa/ssa-dse-24.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct printf_info \\\\*\\\\)&info \\\\+ [0-9]+B\\\\] = {}" 1
>>
>> on 32 and 64-bit sparc-sun-solaris2.11 (the first of those also on
>> m68k-unknown-linux-gnu).

while this group is gone now.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jakub Jelinek]

On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
> >> Even with that fixed, I see many failures:
> >>
> >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
> >>
> >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
> 
> these failures remain...

On i686-linux I can reproduce just the above ones.
The following should fix it.  As we don't match exact offset on the MEM
because it varries between different architectures (24 bytes on with -m64,
28 bytes with -m32), we shouldn't match the store size either, as it is
200 - that offset, so 176 or 172 etc.

Tested on x86_64-linux, -m32/-m64, ok for trunk?

2019-06-13  Jakub Jelinek  <jakub@redhat.com>

	* g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
	= {} store.
	* g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
	-fdump-tree-forwprop1 in dg-options.  Expect <int[5]> in MEM.

--- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj	2019-06-13 00:35:49.654840275 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C	2019-06-13 12:40:14.492568336 +0200
@@ -98,4 +98,4 @@ int main()
 
 
 /* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
-   { dg-final { scan-tree-dump-times "MEM <char\\\[176]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
+   { dg-final { scan-tree-dump-times "MEM <char\\\[\[0-9\]+]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
--- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj	2015-05-29 15:04:33.039803414 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C	2019-06-13 12:24:15.895576933 +0200
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O -fdump-tree-forwprop" } */
+/* { dg-options "-O -fdump-tree-forwprop1" } */
 
 /* We should be able to optimize this to i[j] = 1 during
    early optimizations.  */
@@ -12,4 +12,4 @@ void foo (int j)
   *q = 1;
 }
 
-/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */
+/* { dg-final { scan-tree-dump "MEM <int\\\[5\\\]> \\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */


	Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Rainer Orth]

Hi Jakub,

> On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
>> >> Even with that fixed, I see many failures:
>> >>
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
>> >> "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
>> >> "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
>> >> "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
>> >> <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+
>> >> [0-9]+B\\\\] = {}" 1
>> >>
>> >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
>> 
>> these failures remain...
>
> On i686-linux I can reproduce just the above ones.

right: I'm seeing the rest on sparc-sun-solaris2.11 only (and some of
those also on a couple of other targets according to gcc-testresults).

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Thu, Jun 13, 2019 at 12:45 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
> > >> Even with that fixed, I see many failures:
> > >>
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
> > >>
> > >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
> >
> > these failures remain...
>
> On i686-linux I can reproduce just the above ones.
> The following should fix it.  As we don't match exact offset on the MEM
> because it varries between different architectures (24 bytes on with -m64,
> 28 bytes with -m32), we shouldn't match the store size either, as it is
> 200 - that offset, so 176 or 172 etc.
>
> Tested on x86_64-linux, -m32/-m64, ok for trunk?

OK.

Richard.

> 2019-06-13  Jakub Jelinek  <jakub@redhat.com>
>
>         * g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
>         = {} store.
>         * g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
>         -fdump-tree-forwprop1 in dg-options.  Expect <int[5]> in MEM.
>
> --- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj        2019-06-13 00:35:49.654840275 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C   2019-06-13 12:40:14.492568336 +0200
> @@ -98,4 +98,4 @@ int main()
>
>
>  /* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
> -   { dg-final { scan-tree-dump-times "MEM <char\\\[176]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> +   { dg-final { scan-tree-dump-times "MEM <char\\\[\[0-9\]+]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> --- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj  2015-05-29 15:04:33.039803414 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C     2019-06-13 12:24:15.895576933 +0200
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-forwprop" } */
> +/* { dg-options "-O -fdump-tree-forwprop1" } */
>
>  /* We should be able to optimize this to i[j] = 1 during
>     early optimizations.  */
> @@ -12,4 +12,4 @@ void foo (int j)
>    *q = 1;
>  }
>
> -/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "MEM <int\\\[5\\\]> \\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */
>
>
>         Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

On 6/13/19 4:44 AM, Jakub Jelinek wrote:
> On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
>>>> Even with that fixed, I see many failures:
>>>>
>>>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>>>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>>>> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
>>>> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
>>>>
>>>> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
>>
>> these failures remain...
> 
> On i686-linux I can reproduce just the above ones.
> The following should fix it.  As we don't match exact offset on the MEM
> because it varries between different architectures (24 bytes on with -m64,
> 28 bytes with -m32), we shouldn't match the store size either, as it is
> 200 - that offset, so 176 or 172 etc.
> 
> Tested on x86_64-linux, -m32/-m64, ok for trunk?
> 
> 2019-06-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
> 	= {} store.
> 	* g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
> 	-fdump-tree-forwprop1 in dg-options.  Expect <int[5]> in MEM.
> 
> --- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj	2019-06-13 00:35:49.654840275 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C	2019-06-13 12:40:14.492568336 +0200
> @@ -98,4 +98,4 @@ int main()
>   
>   
>   /* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
> -   { dg-final { scan-tree-dump-times "MEM <char\\\[176]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> +   { dg-final { scan-tree-dump-times "MEM <char\\\[\[0-9\]+]> \\\[\\(struct FixBuf \\*\\)&<retval> \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> --- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj	2015-05-29 15:04:33.039803414 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C	2019-06-13 12:24:15.895576933 +0200
> @@ -1,5 +1,5 @@
>   /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-forwprop" } */
> +/* { dg-options "-O -fdump-tree-forwprop1" } */
>   
>   /* We should be able to optimize this to i[j] = 1 during
>      early optimizations.  */
> @@ -12,4 +12,4 @@ void foo (int j)
>     *q = 1;
>   }
>   
> -/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "MEM <int\\\[5\\\]> \\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */

Thanks for cleaning this up.

The size of the access above doesn't look right.  The test is:

   int i[5];
   void foo (int j)
   {
     void *p = &i[j];
     int *q = (int *)p;
     *q = 1;
   }

and the MEM_REF is for the assignment *q = 1.  It assigns a single
int, not the whole array.  The -gimple output looks pretty much
the same:

   __MEM <int[5]> ((int *)&i)[j_1(D)] = 1;

I expected it to mention the size of the access, e.g., like this:

   __MEM <int> ((int *)&i)[j_1(D)] = 1;

It was the entire goal of the change I made to be able to be able
to see the size of the access so it doesn't look like I got it
right and some more tweaking is necessary.  Which also raises
the question: what is the purpose of the MEM_REF type if not to
encode the size/alignment of the access?

Martin

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jakub Jelinek]

On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote:
> The size of the access above doesn't look right.  The test is:

It is correct.  You have MEM <int[5]> [(int *)&i], which is
the same thing as i itself, and on this you apply an ARRAY_REF,
which is printed after it, with index j_1(D).  ARRAY_REF is applied on
arrays and the result type is the array element type, so int in this case.

	Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Martin Sebor]

On 6/13/19 9:34 AM, Jakub Jelinek wrote:
> On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote:
>> The size of the access above doesn't look right.  The test is:
> 
> It is correct.  You have MEM <int[5]> [(int *)&i], which is
> the same thing as i itself, and on this you apply an ARRAY_REF,
> which is printed after it, with index j_1(D).  ARRAY_REF is applied on
> arrays and the result type is the array element type, so int in this case.

Aah, it's two REFs in one.  I misread the array index as being
a part of the MEM_REF operand, like this:

   MEM <int[5]> [((int *)&i)[j_1(D)]] = 1;

I guess I've never noticed this before.  Why is the whole thing
not simplified to an ARRAY_REF?

   i[j_2(D)] = 1;

Martin

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jakub Jelinek]

On Thu, Jun 13, 2019 at 09:50:16AM -0600, Martin Sebor wrote:
> On 6/13/19 9:34 AM, Jakub Jelinek wrote:
> > On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote:
> > > The size of the access above doesn't look right.  The test is:
> > 
> > It is correct.  You have MEM <int[5]> [(int *)&i], which is
> > the same thing as i itself, and on this you apply an ARRAY_REF,
> > which is printed after it, with index j_1(D).  ARRAY_REF is applied on
> > arrays and the result type is the array element type, so int in this case.
> 
> Aah, it's two REFs in one.  I misread the array index as being
> a part of the MEM_REF operand, like this:
> 
>   MEM <int[5]> [((int *)&i)[j_1(D)]] = 1;
> 
> I guess I've never noticed this before.  Why is the whole thing
> not simplified to an ARRAY_REF?
> 
>   i[j_2(D)] = 1;

No idea in this case, though of course there can be other cases e.g.
where the MEM_REF has different number of elements, different element type
etc. from the underlying variable or where the MEM_REF first operand is not
address, but pointer.

	Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener]

On Thu, Jun 13, 2019 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Jun 13, 2019 at 09:50:16AM -0600, Martin Sebor wrote:
> > On 6/13/19 9:34 AM, Jakub Jelinek wrote:
> > > On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote:
> > > > The size of the access above doesn't look right.  The test is:
> > >
> > > It is correct.  You have MEM <int[5]> [(int *)&i], which is
> > > the same thing as i itself, and on this you apply an ARRAY_REF,
> > > which is printed after it, with index j_1(D).  ARRAY_REF is applied on
> > > arrays and the result type is the array element type, so int in this case.
> >
> > Aah, it's two REFs in one.  I misread the array index as being
> > a part of the MEM_REF operand, like this:
> >
> >   MEM <int[5]> [((int *)&i)[j_1(D)]] = 1;
> >
> > I guess I've never noticed this before.  Why is the whole thing
> > not simplified to an ARRAY_REF?
> >
> >   i[j_2(D)] = 1;
>
> No idea in this case, though of course there can be other cases e.g.
> where the MEM_REF has different number of elements, different element type
> etc. from the underlying variable or where the MEM_REF first operand is not
> address, but pointer.

We elide the MEM_REF only if the TBAA type exactly matches the value-type.
Since TBAA-wise an array is the same as the element type (unless the
language says otherwise - see get_alias_set) we could in some cases with
arrays elide it.  It didn't seem worth the extra complexity in the code doing
this, see maybe_canonicalize_mem_ref_addr.  Honza is at the moment
trying to improve access-path disambiguation which runs into related issues.

Richard.

>         Jakub

Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Jan Hubicka]

> > > Aah, it's two REFs in one.  I misread the array index as being
> > > a part of the MEM_REF operand, like this:
> > >
> > >   MEM <int[5]> [((int *)&i)[j_1(D)]] = 1;
> > >
> > > I guess I've never noticed this before.  Why is the whole thing
> > > not simplified to an ARRAY_REF?
> > >
> > >   i[j_2(D)] = 1;
> >
> > No idea in this case, though of course there can be other cases e.g.
> > where the MEM_REF has different number of elements, different element type
> > etc. from the underlying variable or where the MEM_REF first operand is not
> > address, but pointer.
> 
> We elide the MEM_REF only if the TBAA type exactly matches the value-type.
> Since TBAA-wise an array is the same as the element type (unless the
> language says otherwise - see get_alias_set) we could in some cases with
> arrays elide it.  It didn't seem worth the extra complexity in the code doing
> this, see maybe_canonicalize_mem_ref_addr.  Honza is at the moment
> trying to improve access-path disambiguation which runs into related issues.

I have just sent an email with some analysis running into it on vect
testcase.  I think we ought to solve this - it seems to be relatively
common case and my proposed patch makes it worse (and I think for valid
reasons)

Honza
> 
> Richard.
> 
> >         Jakub