This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] Handle LHS zero_extracts in DSE


Hi,

debugging a problem with an older GCC (4.6).  I've seen RTXs using
ZERO_EXTRACT LHS operands on MEMs as in:

(insn 7 6 0 (set (zero_extract:DI (mem/s/c:QI (plus:DI (reg/f:DI 39 virtual-stack-vars)
		    (const_int -5 [0xfffffffffffffffb])) [0+0 S1 A8])
	    (const_int 24 [0x18])
	    (const_int 0 [0]))
	(subreg:DI (reg:SI 45) 0)) t.c:19 -1
     (nil))

occurring with that testcase:

struct a
{
  struct
  {
    char b:1;
  } c;
  char d[3];
  char e;
  struct
  {
  int:1}
};

void
f ()
{
  struct a g = { };
  char h[4] = "foo";
  memcpy (g.d, h, 3);
  g.c.b = 1;
  i (g);
}


The program gets miscompiled with GCC 4.6 since DSE ignores that INSN
when it comes to recording stores. In that particular case it led to
the assignment g.c.b = 1 overwriting parts of the g.d field.

I couldn't reproduce that on recent GCCs. Since that fix from Jakub
I don't get these zero_extract anymore.

Author: jakub
Date: Thu Jan 26 14:09:29 2012
New Revision: 183560

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183560
Log:
		 PR middle-end/51895
		 * expr.c (expand_expr_real_1): Handle BLKmode MEM_REF of
		 non-addressable non-BLKmode base correctly.

		 * g++.dg/opt/pr51895.C: New test.


However, I still think that's a legal RTX which needs to be handled by
DSE correctly. I couldn't find anything preventing that problem from
occuring also with current GCCs. On the other hand I couldn't trigger
it with anything in testsuite.

Perhaps something like that would be needed for current GCC:

diff --git a/gcc/dse.c b/gcc/dse.c
index 21d166d..d27fb54 100644
--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -1346,6 +1346,47 @@ record_store (rtx body, bb_info_t bb_info)

   mem = SET_DEST (body);

+  /* Deal with (zero_extract:XX (mem:QI ...)).  */
+  if (GET_CODE (mem) == ZERO_EXTRACT && MEM_P (XEXP (mem, 0)))
+    {
+      rtx size_op = XEXP (mem, 1);
+      rtx offset_op = XEXP (mem, 2);
+      enum machine_mode mode = GET_MODE (mem);
+      scalar_int_mode int_mode;
+
+      /* Turn a (zero_extract:XX (mem:QI ...)) into a (mem:BLK with
+	 the proper size and the adjusted address.  */
+      if (CONST_INT_P (size_op)
+	  && CONST_INT_P (offset_op)
+	  && is_a <scalar_int_mode> (mode, &int_mode)
+	  && INTVAL (size_op) + INTVAL (offset_op) <= GET_MODE_PRECISION (int_mode))
+	{
+	  int size = INTVAL (size_op);
+	  int offset = INTVAL (offset_op);
+
+	  mem = copy_rtx (XEXP (mem, 0));
+	  if (BITS_BIG_ENDIAN)
+	    mem = adjust_address (mem, BLKmode, offset / BITS_PER_UNIT);
+	  else
+	    mem = adjust_address (mem, BLKmode,
+				  (GET_MODE_BITSIZE (int_mode) - size - offset)
+				  / BITS_PER_UNIT);
+
+	  set_mem_size (mem, (size - 1) / BITS_PER_UNIT + 1);
+	}
+      else
+	{
+	  if (find_reg_note (insn_info->insn, REG_UNUSED, mem) == NULL)
+	    {
+	      /* If the set or clobber is unused, then it does not effect our
+		 ability to get rid of the entire insn.  */
+	      insn_info->cannot_delete = true;
+	      clear_rhs_from_active_local_stores ();
+	    }
+	  return 0;
+	}
+    }
+
   /* If this is not used, then this cannot be used to keep the insn
      from being deleted.  On the other hand, it does provide something
      that can be used to prove that another store is dead.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]