Re: [PATCH v3] Work around the NOP issue of Loongson2F

[Richard Sandiford <rdsandiford at googlemail dot com>]

Wu Zhangjin <wuzhangjin@gmail.com> writes:
>> Why did you remove the md_begin stuff that handled the initialisation
>> of nop_insn?
>
> No, I didn't remove the md_begin() stuff, it's still there to init the
> INSN_NOP.

I'm going blind, sorry.

> On Tue, 2009-11-17 at 18:11 +0800, zhangfx wrote:
> > I think due to the source of the bug, we can leave alone .align, .fill 
> > implementations. Only when they are used in a sequence of short basic 
> > blocks they can be dangerous.
> > 
> > e.g.
> >   beq L1;
> >    nop
> > L1:
> >   beq L2;
> >   nop
> > L2:
> >   beq L3;
> >   nop
> > ...
> > 
>
> Thanks, later, I will send the patch out without considering the .align
> & .fill issue.

Can the issue occur with:

  beq L1;
  any insn <--- note
  nop
L1:
  beq L2;
  nop
L2:
  beq L3;
  nop

?  If so, then we do need to handle the .align case, since it is quite
common to use ".align" before labels.

Even if the answer's no, I'm uncomfortable with one kind of nop being
treated differently.

> "The nature of the erratum is deeply related to the microarchitecture of
> Loongson-2. It uses roughly a 4-way superscalar dynamically scheduled core,
> instructions are excuted as much as possible in parallel with technics like
> branch prediction etc. We use a 8-entry internal branch prediction queue to
> keep track of each predicted branches, if some branches are proved to be
> wrongly predicted, all the instructions following it will be cancelled,together
> with the resources used by them, including the registers used for renaming, and
> the queue entry will be freeed. There is a bug that might cause a hang when the
> queue is full(some resources might been leaked due to conflict branch entries),
> the workaround is to reduce the possiblity of branch queue full by using
> renaming registers(they are also limited, can prevent too many simutaneos
> branches). In theory this is still not enough to fully eliminate possible
> hangs, but the possiblity is extremely low now and hard to be hit in real
> code."

That's a good description of the problem, but I'm still not sure I understand
why using $at (in particular) is the chosen workaround.

> @@ -7361,6 +7367,11 @@ macro (struct mips_cl_insn *ip)
>        move_register (dreg, sreg);
>        break;
>  
> +    case M_NOP:
> +      if (mips_fix_loongson2f)
> +        append_insn (NOP_INSN, NULL, unused_reloc);
> +      break;
> +

This should be unconditional.  "nop" has to expand to something
regardless of whether -mfix-loongson2f is used.

> +00000000 <.text>:
> +.*:	00200825 	move	\$1,\$1
> +.*:	10000001 	b	0xc
> +.*:	00200825 	move	\$1,\$1
> +.*:	00200825 	move	\$1,\$1
> +	...
> +.*:	00200825 	move	\$1,\$1

It isn't really appropriate to replace the final address with ".*" here,
since we want to check that the move is at offset 0x20.

Richard