RFA: Enhance doloop_condition_get()

[Nick Clifton <nickc at redhat dot com>]

Hi Guys,

  Whilst working on a patch to add doloop support for the MN10300 I came
  across a problem with the doloop_condition_get() function in
  loop-doloop.c.  Specifically, it would accept a sequence like this:

    (set (reg) (plus (reg) (const_int -1)))
    (set (pc)  (if_then_else (reg != 0) (label_ref (label)) (pc)))

  But not one like this:

    [(set (reg) (plus (reg) (const_int -1)))
     (set (cc)  (compare (plus (reg) (const_int -1)) (const_int 0)))]
    (set (pc)  (if_then_else (reg != 0) (label_ref (label)) (pc)))

  ie a sequence in which the increment instruction explicitly sets the
  condition code register.

  So I fixed that, but in the course of doing so I decided to tidy up
  the function a little bit further by:

    * Using prev_nondebug_nonnote_insn() instead of PREV_INSN.
    
    * Adding entries to the dump file (if enabled) to help track down
      why a doloop pattern does not satisfy doloop_condition_get().

  I was also tempted to allow the function to accept:

    (minus (reg) (const_int 1))

  As the decrement operation, since that makes more sense to me than:

    (plus (reg) (const_int -1))

  But I decided to leave well enough alone.

  In the course of testing the patch however I ran across a small bug in
  web.c:union_match_dups() when it was processing these doloop
  patterns.  Under some circumstances the loop that walks the ref list
  could reach the end of the list leaving a NULL pointer, but then the
  code would go on to try to dereference that pointer.  So this patch
  includes the obvious fix for that problem too.

  Tested with a i686-pc-linux-gnu toolchain as well as an mep-elf
  toolchain (and an mn10300-elf toolchain with a local doloop patch
  applied).

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2011-02-22  Nick Clifton  <nickc@redhat.com>

        * loop-doloop.c (doloop_condition_get): Use
	prev_nonnote_nondebug_insn.  Add diagnostics for why a doloop
	pattern does not satisfy doloop_condition_get.  Allow the
	increment insn to be a parallel of a SET and some other action.
          
	* web.c (union_match_dups): Allow for reaching the end of the
	reference list.

Index: gcc/loop-doloop.c
===================================================================
--- gcc/loop-doloop.c	(revision 170392)
+++ gcc/loop-doloop.c	(working copy)
@@ -103,20 +103,32 @@
   if (GET_CODE (pattern) != PARALLEL)
     {
       rtx cond;
-      rtx prev_insn = prev_nondebug_insn (doloop_pat);
+      rtx prev_insn = prev_nonnote_nondebug_insn (doloop_pat);
 
       /* We expect the decrement to immediately precede the branch.  */
-
       if (prev_insn == NULL_RTX || !INSN_P (prev_insn))
-        return 0;
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Doloop pattern check fail: no previous instruction\n");
+	  
+	  return NULL_RTX;
+	}
 
       cmp = pattern;
-      inc = PATTERN (PREV_INSN (doloop_pat));
+      inc = PATTERN (prev_insn);
       /* We expect the condition to be of the form (reg != 0)  */
       cond = XEXP (SET_SRC (cmp), 0);
       if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
-        return 0;
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Doloop pattern check fail: comparison not (NE 0)\n");
+	  return NULL_RTX;
+	}
 
+      /* Allow for the case where the decrement is in parallel with
+	 (presumably) a clobber or set of the condition code register.  */
+      if (GET_CODE (inc) == PARALLEL)
+	inc = XVECEXP (inc, 0, 0);
     }
   else
     {
@@ -126,20 +138,34 @@
 
   /* Check for (set (reg) (something)).  */
   if (GET_CODE (inc) != SET)
-    return 0;
+    {
+      if (dump_file)
+	fprintf (dump_file, "Doloop pattern check fail: no decrement insn\n");
+      return NULL_RTX;
+    }
+
   reg = SET_DEST (inc);
   if (! REG_P (reg))
-    return 0;
+    {
+      if (dump_file)
+	fprintf (dump_file, "Doloop pattern check fail: no decrement of loop counter\n");
+      return NULL_RTX;
+    }
 
   /* Check if something = (plus (reg) (const_int -1)).
      On IA-64, this decrement is wrapped in an if_then_else.  */
   inc_src = SET_SRC (inc);
   if (GET_CODE (inc_src) == IF_THEN_ELSE)
     inc_src = XEXP (inc_src, 1);
+  /* FIXME: We should also accept (MINUS (reg) (const_int 1)) here... */
   if (GET_CODE (inc_src) != PLUS
       || XEXP (inc_src, 0) != reg
       || XEXP (inc_src, 1) != constm1_rtx)
-    return 0;
+    {
+      if (dump_file)
+	fprintf (dump_file, "Doloop pattern check fail: decrement not (PLUS -1)\n");
+    return NULL_RTX;
+    }
 
   /* Check for (set (pc) (if_then_else (condition)
                                        (label_ref (label))
@@ -149,7 +175,11 @@
       || GET_CODE (SET_SRC (cmp)) != IF_THEN_ELSE
       || GET_CODE (XEXP (SET_SRC (cmp), 1)) != LABEL_REF
       || XEXP (SET_SRC (cmp), 2) != pc_rtx)
-    return 0;
+    {
+      if (dump_file)
+	fprintf (dump_file, "Doloop pattern check fail: no test of loop counter\n");
+    return NULL_RTX;
+    }
 
   /* Extract loop termination condition.  */
   condition = XEXP (SET_SRC (cmp), 0);
@@ -159,7 +189,11 @@
        && GET_CODE (condition) != NE)
       || (XEXP (condition, 1) != const0_rtx
           && XEXP (condition, 1) != const1_rtx))
-    return 0;
+    {
+      if (dump_file)
+	fprintf (dump_file, "Doloop pattern check fail: comparison not GE/NE\n");
+      return NULL_RTX;
+    }
 
   if ((XEXP (condition, 0) == reg)
       || (GET_CODE (XEXP (condition, 0)) == PLUS
@@ -181,17 +215,15 @@
                      (set (reg) (plus (reg) (const_int -1)))
                      (additional clobbers and uses)])
 
-         So we return that form instead.
-     */
+         So we return that form instead.  */
         condition = gen_rtx_fmt_ee (NE, VOIDmode, inc_src, const1_rtx);
 
-    return condition;
+     return condition;
    }
 
-  /* ??? If a machine uses a funny comparison, we could return a
-     canonicalized form here.  */
-
-  return 0;
+  if (dump_file)
+    fprintf (dump_file, "Doloop pattern: condition does not check loop counter\n");
+  return NULL_RTX;
 }
 
 /* Return nonzero if the loop specified by LOOP is suitable for
Index: gcc/web.c
===================================================================
--- gcc/web.c	(revision 170392)
+++ gcc/web.c	(working copy)
@@ -122,6 +122,8 @@
       for (; *ref; ref++)
 	if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
 	  break;
+      if (*ref == NULL)
+	continue;
 
       (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
     }