diff mbox

Fix var-tracking.c (adjust_insn) on inline-asm with multiple outputs (PR debug/45015)

Message ID 20100721143739.GN19172@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek July 21, 2010, 2:37 p.m. UTC 
Hi!

adjust_insns doesn't handle correctly inline-asm with multiple sets.
In that case PATTERN is a PARALLEL with a bunch of SETs from ASM_OPERANDS
(and optionally some CLOBBERs), but the 3 vector operands of ASM_OPERANDS
rtx must be shared between all of them in the same insn.
adjust_insn would instead walk the inputs and do any needed changes in
all of them separately.  This means the vectors weren't shared and, if any
of the operands had any side-effects, the side-effects would be accounted
several times (which leads to ICEs).

The following patch fixes that by only ever changing operands of
ASM_OPERANDS for the first ASM_OPERANDS in the instruction, and by ensuring
the vectors are shared afterwards as well in adjust_insn.

Bootstrapped/regtested on x86_64-linux and i686-linux (and tested with the
testcase on m68k-linux cross).  Ok for trunk?  What about 4.5, which has the
same bug?

2010-07-21  Jakub Jelinek  <jakub@redhat.com>

	PR debug/45015
	* var-tracking.c (adjust_mems): Ignore ASM_OPERANDS with non-zero
	ASM_OPERANDS_OUTPUT_IDX.
	(adjust_insn): For inline asm with multiple sets ensure first
	ASM_OPERANDS vectors are used by all following ASM_OPERANDS in
	the insn.

	* gcc.target/m68k/pr45015.c: New test.


	Jakub

Comments

Richard Henderson July 21, 2010, 5:54 p.m. UTC | #1 
On 07/21/2010 07:37 AM, Jakub Jelinek wrote:
> +#ifdef ENABLE_CHECKING
> +      gcc_assert (GET_CODE (set0) == SET
> +		  && GET_CODE (SET_SRC (set0)) == ASM_OPERANDS
> +		  && ASM_OPERANDS_OUTPUT_IDX (SET_SRC (set0)) == 0);
> +#endif

gcc_checking_assert for mainline.

Otherwise ok everywhere.


r~
diff mbox

Patch

--- gcc/var-tracking.c.jj	2010-07-21 09:53:10.000000000 +0200
+++ gcc/var-tracking.c	2010-07-21 14:04:26.000000000 +0200
@@ -910,6 +910,16 @@  adjust_mems (rtx loc, const_rtx old_rtx,
 	return use_narrower_mode (SUBREG_REG (tem), GET_MODE (tem),
 				  GET_MODE (SUBREG_REG (tem)));
       return tem;
+    case ASM_OPERANDS:
+      /* Don't do any replacements in second and following
+	 ASM_OPERANDS of inline-asm with multiple sets.
+	 ASM_OPERANDS_INPUT_VEC, ASM_OPERANDS_INPUT_CONSTRAINT_VEC
+	 and ASM_OPERANDS_LABEL_VEC need to be equal between
+	 all the ASM_OPERANDs in the insn and adjust_insn will
+	 fix this up.  */
+      if (ASM_OPERANDS_OUTPUT_IDX (loc) != 0)
+	return loc;
+      break;
     default:
       break;
     }
@@ -960,7 +970,57 @@  adjust_insn (basic_block bb, rtx insn)
   note_stores (PATTERN (insn), adjust_mem_stores, &amd);
 
   amd.store = false;
-  note_uses (&PATTERN (insn), adjust_mem_uses, &amd);
+  if (GET_CODE (PATTERN (insn)) == PARALLEL
+      && asm_noperands (PATTERN (insn)) > 0
+      && GET_CODE (XVECEXP (PATTERN (insn), 0, 0)) == SET)
+    {
+      rtx body, set0;
+      int i;
+
+      /* inline-asm with multiple sets is tiny bit more complicated,
+	 because the 3 vectors in ASM_OPERANDS need to be shared between
+	 all ASM_OPERANDS in the instruction.  adjust_mems will
+	 not touch ASM_OPERANDS other than the first one, asm_noperands
+	 test above needs to be called before that (otherwise it would fail)
+	 and afterwards this code fixes it up.  */
+      note_uses (&PATTERN (insn), adjust_mem_uses, &amd);
+      body = PATTERN (insn);
+      set0 = XVECEXP (body, 0, 0);
+#ifdef ENABLE_CHECKING
+      gcc_assert (GET_CODE (set0) == SET
+		  && GET_CODE (SET_SRC (set0)) == ASM_OPERANDS
+		  && ASM_OPERANDS_OUTPUT_IDX (SET_SRC (set0)) == 0);
+#endif
+      for (i = 1; i < XVECLEN (body, 0); i++)
+	if (GET_CODE (XVECEXP (body, 0, i)) != SET)
+	  break;
+	else
+	  {
+	    set = XVECEXP (body, 0, i);
+#ifdef ENABLE_CHECKING
+	    gcc_assert (GET_CODE (SET_SRC (set)) == ASM_OPERANDS
+			&& ASM_OPERANDS_OUTPUT_IDX (SET_SRC (set)) == i);
+#endif
+	    if (ASM_OPERANDS_INPUT_VEC (SET_SRC (set))
+		!= ASM_OPERANDS_INPUT_VEC (SET_SRC (set0))
+		|| ASM_OPERANDS_INPUT_CONSTRAINT_VEC (SET_SRC (set))
+		   != ASM_OPERANDS_INPUT_CONSTRAINT_VEC (SET_SRC (set0))
+		|| ASM_OPERANDS_LABEL_VEC (SET_SRC (set))
+		   != ASM_OPERANDS_LABEL_VEC (SET_SRC (set0)))
+	      {
+		rtx newsrc = shallow_copy_rtx (SET_SRC (set));
+		ASM_OPERANDS_INPUT_VEC (newsrc)
+		  = ASM_OPERANDS_INPUT_VEC (SET_SRC (set0));
+		ASM_OPERANDS_INPUT_CONSTRAINT_VEC (newsrc)
+		  = ASM_OPERANDS_INPUT_CONSTRAINT_VEC (SET_SRC (set0));
+		ASM_OPERANDS_LABEL_VEC (newsrc)
+		  = ASM_OPERANDS_LABEL_VEC (SET_SRC (set0));
+		validate_change (NULL_RTX, &SET_SRC (set), newsrc, true);
+	      }
+	  }
+    }
+  else
+    note_uses (&PATTERN (insn), adjust_mem_uses, &amd);
 
   /* For read-only MEMs containing some constant, prefer those
      constants.  */
--- gcc/testsuite/gcc.target/m68k/pr45015.c.jj	2010-07-21 14:28:14.000000000 +0200
+++ gcc/testsuite/gcc.target/m68k/pr45015.c	2010-07-21 14:29:46.000000000 +0200
@@ -0,0 +1,26 @@ 
+/* PR debug/45015 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+unsigned int
+foo (unsigned int *x, const unsigned int *y, int z, unsigned int w)
+{
+  unsigned int a, b, c, s;
+  int j;
+  j = -z;
+  x -= j;
+  y -= j;
+  a = 0;
+  do
+    {
+      __asm__ ("move.l %2, %0; move.l %3, %1" : "=d" (b), "=d" (c) : "g<>" (y[j]), "d" (w));
+      c += a;
+      a = (c < a) + b;
+      s = x[j];
+      c = s + c;
+      a += (c < s);
+      x[j] = c;
+    }
+  while (++j != 0);
+  return a;
+}