Your Jan 8th, 2001 rtlanal.c:note_stores change

[David S. Miller]

Richard, pieces of the following change:

Thu Jan 18 06:43:04 2001  Richard Kenner  <kenner@vlsi1.ultra.nyu.edu>

        * flow.c (mark_set_1, case PARALLEL): New case; rework to allow
        entry to be EXPR_LIST.
        * rtlanal.c (reg_overlap_mentioned_p): Allow PARALLEL in SET to
        be an EXPR_LIST (but not null, which other code doesn't allow).
        (note_stores): Properly handle PARALLEL in SET.
        Recursively call for top-level PARALLEL.
        * sched-deps.c (sched_analyze_1): Handle EXPR_LIST in PARALLEL in SET.
        * sched-rgn.c (check_live_1, update_live_1): Likewise.

In particular, the note_stores change, breaks a lot of things on
platforms that return values from functions in multiple registers
such as Sparc64 and some MIPS configurations.  Amusingly, in your
posting to gcc-patches on Jan 18th, 2001, mentioning this patch,
it is said that these changes fix a bug on sparc64. :-)

(to see the note_stores changes in question, diff revisions 1.81 and
 1.82 of gcc/rtlanal.c in CVS)

You can't pass the pieces of the PARALLEL SET_DEST as a CLOBBER to the
note_stores callback function.  Do you understand how these are now
being interpreted?  They need to be SETs so that things like live
register information will be updated properly.

These pieces are being SET, not CLOBBERed.  Why did you make this
a CLOBBER?  What broke by it being 'x', the real SET?

A CLOBBERed object has no useful value after the operation, a SET
does.  These registers become live after the operation, so they must
be shown to the note_stores callback as being SET.

The example case I hit was dbr_schedule() seeing bad live register
information over a call that looked like this:

(call_insn:TI 142 141 156 (parallel[
            (set (parallel:BLK[
                        (expr_list (reg:DI 8 %o0)
                            (const_int 0 [0x0]))
                        (expr_list (reg:DI 9 %o1)
                            (const_int 8 [0x8]))
                    ] )
                (call (mem:DI (symbol_ref:DI ("decode_line_spec_1")) [0 S8 A64])
                    (const_int 0 [0x0])))
            (clobber (reg:DI 15 %o7))
        ] ) 337 {*call_value_symbolic_sp64} (insn_list 141 (nil))
    (expr_list:REG_UNUSED (reg:DI 15 %o7)
        (nil))
    (expr_list (use (reg:DI 9 %o1))
        (expr_list (use (reg:DI 8 %o0))
            (nil))))

Liveness in this case is computed by

resources.c:mark_target_live_regs()

which uses:

              note_stores (PATTERN (real_insn), update_live_status, NULL);

And upon seeing this call it saw the bogus CLOBBER of these return
values, so update_live_status correctly interpreted what it was given
by doing this:

  if (GET_CODE (x) == CLOBBER)
    for (i = first_regno; i < last_regno; i++)
      CLEAR_HARD_REG_BIT (current_live_regs, i);

%o0 and %o1 are marked as not live and everything basically explodes
after this point.

The end result was dbr_schedule() not setting the annul bit on a
delay-slot branch, causing %o0 to be incorrectly SET in the
fallthrough case.

The rest of the change, besides this note_stores CLOBBER thing, looks
OK and indeed they do fix bugs in liveness calculations when seeing
such PARALLEL expressions.  Only this note_stores CLOBBER bit is
mysterious.

I'm running a bootstrap + make check on sparc64-linux-gnu with the
following change, which is what I think the correct behavior is.
It fixes the test case, and there are no regressions so far...

--- rtlanal.c.~1~	Wed Apr 10 19:54:40 2002
+++ rtlanal.c	Mon Apr 15 05:36:27 2002
@@ -1542,17 +1542,12 @@
 	dest = XEXP (dest, 0);
 
       /* If we have a PARALLEL, SET_DEST is a list of EXPR_LIST expressions,
-	 each of whose first operand is a register.  We can't know what
-	 precisely is being set in these cases, so make up a CLOBBER to pass
-	 to the function.  */
+	 each of whose first operand is a register.  */
       if (GET_CODE (dest) == PARALLEL)
 	{
 	  for (i = XVECLEN (dest, 0) - 1; i >= 0; i--)
 	    if (XEXP (XVECEXP (dest, 0, i), 0) != 0)
-	      (*fun) (XEXP (XVECEXP (dest, 0, i), 0),
-		      gen_rtx_CLOBBER (VOIDmode,
-				       XEXP (XVECEXP (dest, 0, i), 0)),
-		      data);
+	      (*fun) (XEXP (XVECEXP (dest, 0, i), 0), x, data);
 	}
       else
 	(*fun) (dest, x, data);