[PATCH] Fix DSE not to consider calls as reads from function's body (PR target/77834)

Jakub Jelinek jakub@redhat.com
Fri Nov 4 16:35:00 GMT 2016


On Fri, Nov 04, 2016 at 05:18:13PM +0100, Bernd Schmidt wrote:
> On 11/04/2016 01:26 PM, Jakub Jelinek wrote:
> >
> >In any case, the second hunk fixes an important DSE bug that just got
> >revealed by the former change.
> 
> Can you elaborate on that (what situation does it occur in, is it covered by
> the testsuite)?

As discussed earlier, I'm proposing to leave the first hunk out and have
bootstrapped/regtested the following on x86_64-linux and i686-linux, ok for
trunk?

I don't have a testcase except for
FAIL: gcc.dg/torture/pr67690.c   -Os  execution test
that fails on i686-linux (or x86_64-linux with -m32) with the other hunk,
the details are mentioned in PR77834 comments.  We don't remove the store of
an argument to sibling constant call during the local DSE phase, and
the const sibling call is marked insn_info->frame_read, because of the
need of the arguments.  During the propagation it is handled right, but then
when doing the last step dse_step5 wouldn't call scan_reads on the sibcall,
because it didn't have non-NULL insn_info->rec nor
insn_info->non_frame_wild_read (the latter is used just for non-const calls
that may read anything).  scan_reads does pretty much:
  read_info_t read_info = insn_info->read_rec;
  if (insn_info->non_frame_wild_read)
    do_something;
  if (insn_info->frame_read)
    do_something2;
  while (read_info)
    do_something3;
so not calling it for the insn_info->frame_read case is clearly wrong.

2016-11-04  Jakub Jelinek  <jakub@redhat.com>

	PR target/77834
	* dse.c (dse_step5): Call scan_reads even if just
	insn_info->frame_read.  Improve and fix dump file messages.

--- gcc/dse.c.jj	2016-11-03 15:52:12.521965058 +0100
+++ gcc/dse.c	2016-11-04 09:42:27.640098125 +0100
@@ -3298,12 +3298,19 @@ dse_step5 (void)
 		  bitmap_clear (v);
 		}
 	      else if (insn_info->read_rec
-                       || insn_info->non_frame_wild_read)
+		       || insn_info->non_frame_wild_read
+		       || insn_info->frame_read)
 		{
-		  if (dump_file && !insn_info->non_frame_wild_read)
-		    fprintf (dump_file, "regular read\n");
-                  else if (dump_file && (dump_flags & TDF_DETAILS))
-		    fprintf (dump_file, "non-frame wild read\n");
+		  if (dump_file && (dump_flags & TDF_DETAILS))
+		    {
+		      if (!insn_info->non_frame_wild_read
+			  && !insn_info->frame_read)
+			fprintf (dump_file, "regular read\n");
+		      if (insn_info->non_frame_wild_read)
+			fprintf (dump_file, "non-frame wild read\n");
+		      if (insn_info->frame_read)
+			fprintf (dump_file, "frame read\n");
+		    }
 		  scan_reads (insn_info, v, NULL);
 		}
 	    }


	Jakub



More information about the Gcc-patches mailing list