public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
Date: Sat, 01 Jun 2019 16:16:00 -0000	[thread overview]
Message-ID: <6f658a37-2b2f-1c8b-9a46-c7064530d29e@gmail.com> (raw)
In-Reply-To: <ecf4c116-9697-71c7-c04f-d563b8c1f64d@gmail.com>

[-- 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)

  reply	other threads:[~2019-06-01 16:16 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-01 15:53 Martin Sebor
2019-06-01 16:16 ` Martin Sebor [this message]
2019-06-03  8:36 ` Richard Biener
2019-06-03  8:57   ` Jakub Jelinek
2019-06-03 10:35     ` Richard Biener
2019-06-03 15:13       ` Martin Sebor
2019-06-04 10:58         ` Richard Biener
2019-06-10 19:23           ` Martin Sebor
2019-06-10 19:29             ` Jakub Jelinek
2019-06-10 20:37               ` Martin Sebor
2019-06-11 10:43                 ` Richard Biener
2019-06-12 21:34                   ` Rainer Orth
2019-06-12 21:47                     ` Martin Sebor
2019-06-13  9:04                       ` Rainer Orth
2019-06-13 10:45                         ` Jakub Jelinek
2019-06-13 11:18                           ` Richard Biener
2019-06-13 11:18                           ` Rainer Orth
2019-06-13 15:30                           ` Martin Sebor
2019-06-13 15:34                             ` Jakub Jelinek
2019-06-13 15:50                               ` Martin Sebor
2019-06-13 15:54                                 ` Jakub Jelinek
2019-06-14  7:44                                   ` Richard Biener
2019-06-14  8:51                                     ` Jan Hubicka

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=6f658a37-2b2f-1c8b-9a46-c7064530d29e@gmail.com \
    --to=msebor@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

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

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