Hi!

One of the common reasons why nonoverlapping_memrefs_p is called with
MEM_EXPRs that are FUNCTION_DECLs and don't have DECL_RTL set is that
DSE considers all calls to be memory reads from what is being called.

The following patch avoids that.  Of course the question is if we want
to support something like:
char var[64];
...
var[0] = '0xe8';
var[1] = ...;
(void (*fn) (void) = (void (*) (void)) &var[0];
fn ();
without any kind of barrier, or not.
If we do, then the patch below might break it by DSEing the stores.
So, do we want the patch as is, or just limit it to direct function calls
(call (mem (symbol_ref))), or only those where the symbol is actually
a FUNCTION_DECL?  Or do we want to consider arbitrary pointers possibly
writing into function function text?

In any case, the second hunk fixes an important DSE bug that just got
revealed by the former change.

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

        PR target/77834
        * dse.c (check_mem_read_use): For CALL with MEM as first operand,
        ignore that MEM, but recurse on the MEM's operand if not SYMBOL_REF.
        (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
@@ -2159,6 +2159,19 @@ check_mem_read_use (rtx *loc, void *data
       rtx *loc = *iter;
       if (MEM_P (*loc))
        check_mem_read_rtx (loc, (bb_info_t) data);
+      else if (GET_CODE (*loc) == CALL && MEM_P (XEXP (*loc, 0)))
+       {
+         /* Ignore the MEM in first operand of CALL, that isn't
+            any kind of read of the function text memory.  */
+         iter.skip_subrtxes ();
+         /* But for indirect function calls consider the MEM
+            containing the function pointer if any.  Avoid recursing
+            in the most common case.  */
+         if (GET_CODE (XEXP (XEXP (*loc, 0), 0)) != SYMBOL_REF)
+           check_mem_read_use (&XEXP (XEXP (*loc, 0), 0), data);
+         if (!CONST_INT_P (XEXP (*loc, 1)))
+           check_mem_read_use (&XEXP (*loc, 1), data);
+       }
     }
 }
 
@@ -3298,12 +3311,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

Reply via email to