[mips] Micromips delay slot fix

We found a bug in the micromips implementation where the branch delay slot
was not getting filled for micromips.  You can reproduce this with a program
that just has an empty main function.  Andrew Bennett created this fix for
it and we would like to check it in.  I am submitting it for Andrew since he
doesn't have write access.

OK to checkin?

2013-06-10  Andrew Bennett <andrew.bennett@imgtec.com>
	    Steve Ellcey  <sellcey@mips.com>

	* config/mips/mips.md (single_insn): Fix attribute for micromips.

On Mon, 10 Jun 2013, Steve Ellcey  wrote:

> We found a bug in the micromips implementation where the branch delay slot
> was not getting filled for micromips.  You can reproduce this with a program
> that just has an empty main function.  Andrew Bennett created this fix for
> it and we would like to check it in.  I am submitting it for Andrew since he
> doesn't have write access.

 Your fix aside shouldn't empty main expand simply to:

        jrc     $ra

?  There's no delay slot there.

> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> index 2fdc79d..f18ab50 100644
> --- a/gcc/config/mips/mips.md
> +++ b/gcc/config/mips/mips.md
> @@ -704,7 +704,9 @@
>  
>  ;; Is it a single instruction?
>  (define_attr "single_insn" "no,yes"
> -  (symbol_ref "(get_attr_length (insn) == (TARGET_MIPS16 ? 2 : 4)
> +  (symbol_ref "(((TARGET_MIPS16 || TARGET_MICROMIPS)
> +		 && get_attr_length (insn) == 2)
> +		|| (!TARGET_MIPS16 && get_attr_length (insn) == 4)
>  		? SINGLE_INSN_YES : SINGLE_INSN_NO)"))

 Is it safe to assume an RTL insn whose length is 4 has only a single 
machine instruction in the microMIPS mode?  What's the difference to the 
MIPS16 instruction set here?

  Maciej

On Mon, 2013-06-10 at 23:13 +0100, Maciej W. Rozycki wrote:
> On Mon, 10 Jun 2013, Steve Ellcey  wrote:
> 
> > We found a bug in the micromips implementation where the branch delay slot
> > was not getting filled for micromips.  You can reproduce this with a program
> > that just has an empty main function.  Andrew Bennett created this fix for
> > it and we would like to check it in.  I am submitting it for Andrew since he
> > doesn't have write access.
> 
>  Your fix aside shouldn't empty main expand simply to:
> 
>         jrc     $ra
> 
> ?  There's no delay slot there.

Sorry, it should have been 'main(void) {return 0; }.  Then you get (with
the patch):

	j	$31
	move	$2,$0

instead of:

	move	$2,$0
	j	$31


> > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
> > index 2fdc79d..f18ab50 100644
> > --- a/gcc/config/mips/mips.md
> > +++ b/gcc/config/mips/mips.md
> > @@ -704,7 +704,9 @@
> >  
> >  ;; Is it a single instruction?
> >  (define_attr "single_insn" "no,yes"
> > -  (symbol_ref "(get_attr_length (insn) == (TARGET_MIPS16 ? 2 : 4)
> > +  (symbol_ref "(((TARGET_MIPS16 || TARGET_MICROMIPS)
> > +		 && get_attr_length (insn) == 2)
> > +		|| (!TARGET_MIPS16 && get_attr_length (insn) == 4)
> >  		? SINGLE_INSN_YES : SINGLE_INSN_NO)"))
> 
>  Is it safe to assume an RTL insn whose length is 4 has only a single 
> machine instruction in the microMIPS mode?  What's the difference to the 
> MIPS16 instruction set here?
> 
>   Maciej

I think so, I don't know of any microMIPS RTL instructions whose length
is 4 that would not be a single instruction.  Maybe I should add
Catherine in to the discussion to see if she knows differently.

Steve Ellcey
sellcey@mips.com

On Tue, 11 Jun 2013, Steve Ellcey wrote:

> Sorry, it should have been 'main(void) {return 0; }.  Then you get (with
> the patch):
> 
> 	j	$31
> 	move	$2,$0
> 
> instead of:
> 
> 	move	$2,$0
> 	j	$31

 Hmm, something must have been missed then from the microMIPS patches as 
our toolchain seems to get this case right:

	.set	noreorder
	.set	nomacro
	jr	$31
	move	$2,$0

	.set	macro
	.set	reorder

(no idea where the extraneous newline comes from, hmm).  Catherine, can 
you please double-check you haven't got anything outstanding yet?

> >  Is it safe to assume an RTL insn whose length is 4 has only a single 
> > machine instruction in the microMIPS mode?  What's the difference to the 
> > MIPS16 instruction set here?
> 
> I think so, I don't know of any microMIPS RTL instructions whose length
> is 4 that would not be a single instruction.  Maybe I should add
> Catherine in to the discussion to see if she knows differently.

 I advise care here, even if we currently have no such pattern.  It's easy 
to miss the dependency and any such bug introduced could trigger very 
rarely only.  Also (unlike with the standard ISA) some otherwise ordinary 
instructions can't be scheduled into a delay slot, e.g. MOVEP or 
LWP/LDP/SWP/SDP.

  Maciej