This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] MIPS 24K Errata Patch

On 04/19/2011 03:41 PM, Richard Sandiford wrote:
Catherine Moore<> writes:
I've attached updated patches for the testsuite and the assembler.  I
think that I've addressed all of your comments.  Do these new patches
now look okay?

Thanks for the updates. They look good apart from a few minor stylistic nits. I'm still not sure about the last point though:

[ Hmm, I see I got the hex offsets wrong last time, sorry.  I realise
   it probably didn't make much sense with those mistakes. ]

  The last case is interesting.  Does the errata still trigger if
  a suxc1 instruction straddles a cache boundary?  E.g., playing
  battleships for a moment:

      0 2 4 6
    0 ........
    8 ........
   10 ........
   18 ....XXXX   suxc1 to 0x1C
   20 XXXX....
   28 ........
   30 ..XX....   sh to 0x32
   38 ......XX   sh to 0x3E

  It looks like the range checks will return 0 in this case, even though
  all three stores write to different doublewords of the second cache line.

Is there any instruction besides suxc1 that is of concern (ie. any other instruction that allows for unaligned access)? I've altered the original patch to insert nops for the case where the offset field is a register. suxc1 is an instruction in this category.

For this test case:
	add $4, $4, $4
	add $4, $4, $4
	add $4, $4, $4
	add $4, $4, $4
	suxc1 $f4, $5($2)
	sh    $4, 0x32($2)
	sh    $4, 0x3e($2)

assembled with -mips32r2 -mfix-24k, The current implementation gives me:

00000000 <.text>:
   0:	00842020 	add	a0,a0,a0
   4:	00842020 	add	a0,a0,a0
   8:	00842020 	add	a0,a0,a0
   c:	00842020 	add	a0,a0,a0
  10:	4c45200d 	suxc1	$f4,a1(v0)
  14:	a4440032 	sh	a0,50(v0)
  18:	00000000 	nop
  1c:	a444003e 	sh	a0,62(v0)

negating the need to alter the range checks for all of the unaligned cases.

The answer might well be "yes, the errata doesn't trigger for this case". If so, it'd be worth mentioning that in the main comment. The answer affects what the comments above the remaining 24 and 32 checks should be.

What I meant was that:

   pos[0].off == 0x1C
   pos[1].off == 0x32
   pos[2].off == 0x3E

and so:

   if (pos[2].off - pos[1].off>= 24
       || pos[1].off - pos[0].off>= 24
       || pos[2].off - pos[0].off>= 32)
     return 0;

would say that no nops are needed here.  However, all stores are writing
to different doublewords of the second cache line, and from a naive
reading of the the errata description, that means that the errata
could trigger.

Which is right?  If the code is right, then the errata description
needs a bit more detail.  If the naive reading of the errata is right,
then the code needs to change.

I've been told that the naive-reading is correct and that yes, the errata should trigger.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]