[4.2 only] RFA: PR 33848: reload rematerialisation of labels

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

PR 33848, a 4.2 regression on MIPS, is about cases in which we have
a branch like:

    if (foo == &&bar)

and in which the register that holds &&bar is not allocated a hard register.
Reload rematerialises the load of &&bar, first replacing the pseudo register
with the label, then replacing the label with a reload register.  The
second step triggers the following code in reload.c:subst_reloads:

	  /* If we're replacing a LABEL_REF with a register, add a
	     REG_LABEL note to indicate to flow which label this
	     register refers to.  */
	  if (GET_CODE (*r->where) == LABEL_REF
	      && JUMP_P (insn))
	    {
	      REG_NOTES (insn) = gen_rtx_INSN_LIST (REG_LABEL,
						    XEXP (*r->where, 0),
						    REG_NOTES (insn));
	      JUMP_LABEL (insn) = XEXP (*r->where, 0);
	   }

So we end up replacing the jump's real JUMP_LABEL with bar, and
cfgcleanup later deletes the real target as dead.

H-P fixed this properly on mainline by splitting REG_LABEL into
REG_LABEL_OPERAND and REG_LABEL_TARGET.  Unfortunately, I think that's
too invasive to backport.  As H-P said in his original posting:

    http://gcc.gnu.org/ml/gcc/2005-12/msg00279.html

it isn't possible in the 4.2 scheme for a jump to have both a target
LABEL_REF and a non-target LABEL_REF, hence his split.  And since
JUMP_LABEL should now always be valid before reload, I think we should
only add a REG_LABEL note when the label is the same as the current
JUMP_LABEL.  Calling rebuild_jump_labels immediately after reload
would then be a no-op for this particular insn, just as it should be.

I put the testcase in gcc.dg/torture rather than gcc.target/mips because
this could happen for any target with "branch on equal registers"
instructions.  I also used a brute-force test; the one in the PR
happened to pass after the %call16 backport.

Regression-tested on mips-linux-gnu.  OK for 4.2?  If so, should I install
the testcase on mainline too?

Richard


gcc/
	PR rtl-optimization/33848
	* reload.c (subst_reloads): When replacing a LABEL_REF with a
	register, only add a REG_LABEL note if the label is the target
	of the jump.

gcc/testsuite/
	PR rtl-optimization/33848
	* gcc.dg/torture/pr33848.c: New test.

Index: gcc/reload.c
===================================================================
--- gcc/reload.c	2007-10-23 11:04:20.000000000 +0100
+++ gcc/reload.c	2007-10-23 11:58:46.000000000 +0100
@@ -6158,17 +6158,16 @@ #define CHECK_MODF(ARRAY)						\
 	    }
 #endif /* ENABLE_CHECKING */
 
-	  /* If we're replacing a LABEL_REF with a register, add a
-	     REG_LABEL note to indicate to flow which label this
+	  /* If we're replacing a LABEL_REF jump target with a register,
+	     add a REG_LABEL note to indicate to flow which label this
 	     register refers to.  */
 	  if (GET_CODE (*r->where) == LABEL_REF
-	      && JUMP_P (insn))
-	    {
-	      REG_NOTES (insn) = gen_rtx_INSN_LIST (REG_LABEL,
-						    XEXP (*r->where, 0),
-						    REG_NOTES (insn));
-	      JUMP_LABEL (insn) = XEXP (*r->where, 0);
-	   }
+	      && JUMP_P (insn)
+	      && JUMP_LABEL (insn) == XEXP (*r->where, 0)
+	      && !find_reg_note (insn, REG_LABEL, XEXP (*r->where, 0)))
+	    REG_NOTES (insn) = gen_rtx_INSN_LIST (REG_LABEL,
+						  XEXP (*r->where, 0),
+						  REG_NOTES (insn));
 
 	  /* Encapsulate RELOADREG so its machine mode matches what
 	     used to be there.  Note that gen_lowpart_common will
Index: gcc/testsuite/gcc.dg/torture/pr33848.c
===================================================================
--- /dev/null	2007-10-23 10:23:31.552097000 +0100
+++ gcc/testsuite/gcc.dg/torture/pr33848.c	2007-10-23 11:58:07.000000000 +0100
@@ -0,0 +1,43 @@
+/* &&foo should be hoisted, but on most targets, excess register pressure
+   forces it to be rematerialized before "data != &&foo".  On targets that
+   have a "branch if registers are equal" instruction, this leads to the
+   branch having two LABEL_REFs: one for the branch target and one for
+   &&foo.  When reloading &&foo into a register, reload would wrongly
+   say that &&foo was the target of the branch, and the real target would
+   then be removed as dead.  */
+/* { dg-do link } */
+#define NVARS 30
+#define MULTI(X) \
+  X( 0), X( 1), X( 2), X( 3), X( 4), X( 5), X( 6), X( 7), X( 8), X( 9), \
+  X(10), X(11), X(12), X(13), X(14), X(15), X(16), X(17), X(18), X(19), \
+  X(20), X(21), X(22), X(23), X(24), X(25), X(26), X(27), X(28), X(29)
+
+#define DECLARE(INDEX) i##INDEX = gv[INDEX]
+#define COPY(INDEX) gv[INDEX] = i##INDEX
+
+volatile int gv[NVARS];
+void *volatile data;
+
+int
+main (void)
+{
+  __label__ foo;
+
+  if (gv[0] == 1)
+    goto foo;
+  data = &&foo;
+  do
+    {
+      int MULTI (DECLARE);
+      MULTI (COPY);
+      MULTI (COPY);
+      MULTI (COPY);
+      if (data != &&foo)
+	gv[0] = 1;
+      else
+	gv[1] = 2;
+    }
+  while (gv[0] > 0);
+ foo:
+  return 0;
+}