problem with flow and caller_save

[Herman ten Brugge]

Hello,

I discovered a bug in flow.c that caused caller_save to generate bad code.

I did sent this problem about 1/2 year ago but nothing happened until
now so I just resent it as a reminder.
I detected this problem some time after a change to caller-save. The
original mail is probably somewhere stored in the mailing lists.

The program that fails for me is:

typedef struct s { int x, y; } s;

void
ff (s a0, s a1, s a2, s a3, s a4, s a5)
{
  if ( 0
       || a0.x != 4660 || a0.y != 17185
       || a1.x != 9320 || a1.y != 34370
       || a2.x != 13980 || a2.y != 51555
       || a3.x != 18640 || a3.y != 68740
       || a4.x != 23300 || a4.y != 85925
       || a5.x != 27960 || a5.y != 103110)
    abort();
}

int
main ()
{
  s a0, a1, a2, a3, a4, a5;

  a0.x = 4660; a0.y = 17185;
  a1.x = 9320; a1.y = 34370;
  a2.x = 13980; a2.y = 51555;
  a3.x = 18640; a3.y = 68740;
  a4.x = 23300; a4.y = 85925;
  a5.x = 27960; a5.y = 103110;

  ff (a0, a1, a2, a3, a4, a5);
  return(0);
}

When compiling this code for m68k-sun-sunos4.1.1. The assembly code looks
like: (only main)

_main:
	link a6,#-8
	moveml #0x3f3c,sp@-
	movel d0,a6@(-4)	<-- caller save done for d0/d1
	movel d1,a6@(-8)
	jbsr ___main
	movew #9320,a4
	movel #34370,a5
	movew #13980,a2
	movel #51555,a3
	movel #18640,d6
	movel #68740,d7
	movel #23300,d4
	movel #85925,d5
	movel #27960,d2
	movel #103110,d3
				<-- original load d0/d1 removed
				    because of caller-save restore
	movel d3,sp@-
	movel d2,sp@-
	movel d5,sp@-
	movel d4,sp@-
	movel d7,sp@-
	movel d6,sp@-
	movel a3,sp@-
	movel a2,sp@-
	movel a5,sp@-
	movel a4,sp@-
	movel a6@(-4),d0	<-- caller restore done for d0/d1
	movel a6@(-8),d1
	movel d1,sp@-		<-- rubish pushed on stack
	movel d0,sp@-
	jbsr _ff
	clrl d0
	moveml a6@(-48),#0x3cfc
	unlk a6
	rts

The problem is that flow.c marks all the register pairs (struct a0,a1,a2,
a3,a4,a5) as caller-save registers. There are some remarks in flow.c why
this is done:
(line 3210)
  /* Modifying just one hardware register of a multi-reg value
     or just a byte field of a register
     does not mean the value from before this insn is now dead.
     But it does mean liveness of that register at the end of the block
     is significant.

     Within mark_set_1, however, we treat it as if the register is
     indeed modified.  mark_used_regs will, however, also treat this
     register as being used.  Thus, we treat these insns as setting a
     new value for the register as a function of its old value.  This
     cases LOG_LINKS to be made appropriately and this will help combine.  */
(line 3872)
        /* Storing in STRICT_LOW_PART is like storing in a reg
           in that this SET might be dead, so ignore it in TESTREG.
           but in some other ways it is like using the reg.

           Storing in a SUBREG or a bit field is like storing the entire
           register in that if the register's value is not used
           then this SET is not needed.  */
I think flow does the correct thing. Flow only has the information
that a SUBREG is modified it does not know that this SUBREG is
actual two regs on the target.

Now global_alloc tries to find free registers. It can not find a match
for all registers so its set the global variable caller_save_needed.

Caller save will try to save/restore the remaining insn around the
___main function. This fails as shown above.

The patch that I sent 1/2 year ago is below.

	Herman.


1999-05-14 Herman A.J. ten Brugge <Haj.Ten.Brugge@net.HCC.nl>

	* caller-save.c (save_call_clobbered_regs) Prevent incorrect
	code when flow.c marks some registers live across calls which
	in fact do not need to be saved.

--- caller-save.c.org	Fri May 14 18:32:40 1999
+++ caller-save.c	Fri May 14 18:35:19 1999
@@ -363,6 +363,14 @@ save_call_clobbered_regs ()
 		  CLEAR_HARD_REG_SET (referenced_regs);
 		  mark_referenced_regs (PATTERN (insn));
 		  AND_HARD_REG_SET (referenced_regs, hard_regs_saved);
+
+	          /* Set bits for registers set. This is only needed when
+		     flow.c marks some registers live across calls which
+		     in fact do not need to be saved.  */
+	          CLEAR_HARD_REG_SET (this_insn_sets);
+	          note_stores (PATTERN (insn), mark_set_regs);
+	          AND_HARD_REG_SET (this_insn_sets, hard_regs_saved);
+	          IOR_HARD_REG_SET (referenced_regs, this_insn_sets);
 		}
 
 	      for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)