[PATCH] protect MIPS embedded-pic case statement jump delay slots.

[cgd@broadcom.com]

Hi folks,

We recently noticed a bug (in gcc 3.0.3) in which MIPS -membedded-pic
case statement jump delay slots would be incorrectly filled.

The compiler would put the delay slot instruction after the jump
instruction implementing the case statement in the assembly output,
but the two instructions are not ".set noreorder"ed so the assembler
puts a nop in the delay slot.

with gcc configured for mips-elf, the following code:

cat > bug.c << __EOF__
extern int e1(int);

void
fn(int a0)
{
  int i = 0;

  for (;;)
    {
      switch (a0)
	{
	case 2: case 3: case 5: case 7:
	  break;
	default:
	  continue;
	}

      e1(i);
      i++;
    }
}
__EOF__

compiled like:

	mips-elf-gcc -S -O2 -membedded-pic

should show the bug.  The case statement ends up being generated like:

	[ ... ]
	.set	noreorder
	bal	$LS11
	sll	$2,$18,2
$LS11:
	addu	$2,$2,$31
	.set	reorder
	lw	$2,$L11-$LS11($2)
	addu	$2,$2,$31
	j	$2
	move	$4,$17

	.rdata
	.align	3
	.text
$L11:
	.word	$L5-$LS11
	[ more .words of the jump table ]
	[ ... ]


Obviously, that 'move' won't ever be executed.  8-)

The problem is missing "%*" protection in the mips.md patterns that
implement the embedded-pic case statements.

The following patch addresses the problem.  There's a similar %*
before most jumps and branches in mips.md.

(I've done a partial check for missing %*'s in front of other
jumps/branches, and can finish doing that check and report the results
to gcc-bugs or anyplace you'd like if people are interested...  But
i'd like to get this approved first.  8-)


A few notes:

* This has been tested on this example, and in our source tree here
  (3.0.3 + mods).  It's not been extensively tested in the stock GCC,
  and it's not clear that adding a test case will help or actually
  easily detect a regression.  (not sure how to make sure this test or
  another is compiled with -membedded-pic, or to make sure that the
  result is linkable and runnable...)

* I don't think i've actually got write-after-approval permission for
  gcc.  (IIRC, I can technically do it, i.e. I have the relevant group
  perms on the cvs machine, but I don't think i'm "allowed" to.)  So,
  either y'all get to tell me that i can write-after-approval, or
  somebody else gets to check it in for me.  ("Hi Eric!" 8-)

* The patch in:

  http://gcc.gnu.org/ml/gcc-patches/2001-03/msg01654.html

  Is still outstanding and applicable.  We've been using it since a
  bit before it was submitted.  Really, it's the right thing.  8-)


chris
===================================================================
[gcc/ChangeLog]

2002-02-07  Chris Demetriou  <cgd@broadcom.com>

	* config/mips/mips.md (casesi_internal, casesi_internal_di):
	Protect jump delay slot instructions with .set noreorder and
	.set nomacro.

Index: mips.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.md,v
retrieving revision 1.114
diff -u -p -r1.114 mips.md
--- mips.md	2002/02/07 06:09:11	1.114
+++ mips.md	2002/02/08 06:04:06
@@ -9476,7 +9476,7 @@ move\\t%0,%z4\\n\\
    (clobber (reg:SI 31))]
   "TARGET_EMBEDDED_PIC"
   "%(bal\\t%S1\;sll\\t%2,%0,2\\n%~%S1:\;addu\\t%2,%2,$31%)\;\\
-lw\\t%2,%1-%S1(%2)\;addu\\t%2,%2,$31\;j\\t%2"
+lw\\t%2,%1-%S1(%2)\;addu\\t%2,%2,$31\\n\\t%*j\\t%2"
   [(set_attr "type"	"jump")
    (set_attr "mode"	"none")
    (set_attr "length"	"24")])
@@ -9491,7 +9491,7 @@ lw\\t%2,%1-%S1(%2)\;addu\\t%2,%2,$31\;j\
    (clobber (reg:DI 31))]
   "TARGET_EMBEDDED_PIC"
   "%(bal\\t%S1\;sll\\t%2,%0,2\\n%~%S1:\;addu\\t%2,%2,$31%)\;\\
-ld\\t%2,%1-%S1(%2)\;daddu\\t%2,%2,$31\;j\\t%2"
+ld\\t%2,%1-%S1(%2)\;daddu\\t%2,%2,$31\\n\\t%*j\\t%2"
   [(set_attr "type"	"jump")
    (set_attr "mode"	"none")
    (set_attr "length"	"24")])