[RFA] MIPS 24K Errata Patch

[Catherine Moore]

Hi Richard,

On 04/20/2011 04:55 AM, Richard Sandiford wrote:
> Catherine Moore<clm@codesourcery.com>  writes:
>> On 04/19/2011 03:41 PM, Richard Sandiford wrote:
>>> Catherine Moore<clm@codesourcery.com>   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.
>
> Yeah, sorry for the noise.  As is so often the case, it only occured to
> me after signing off that this was now handled earlier.

No problem.  Glad we got it sorted out.


>
>> !   	      /* Don't swap if -mfix-24k and previous insn is a store.  */
>> !   	      || (mips_fix_24k
>> !  		&&  (prev_pinfo&  INSN_STORE_MEMORY)))
>
> This shouldn't be necessary.  It ought to be caught by:
>
> 	      /* Check for conflicts between the swapped sequence and the
> 		 target of the branch.  */
> 	      || nops_for_sequence (2, history + 1, ip, history)>  0
>
> The patch is OK without this hunk, but if you think it's needed,
> let me know.
>
I've removed this, but it required that the 24k-branch-delay test be 
updated.  I agree that it's not necessary.

I've attached the patches that I committed for reference.

Thank you for the thorough review.

Catherine

2011-04-20  Catherine Moore  <clm@codesourcery.com>
             David Ung <davidu@mips.com>

	* config/mips.c (mips_cl_insn): Add new field complete_p.
	(create_insn): Initialize complete_p to zero.
	(BASE_REG_EQ): New.
	(fix_24k_align_to): New.
	(fix_24k_store_info): Declare.
	(fix_24k_sort): New.
	(fix_24k_record_store_info): New.
	(nops_for_24k): New.
	(nops_for_insn): Call nops_for_24k.
	(append_insn): Move O_constant expression handling.

2011-04-20  Catherine Moore  <clm@codesourcery.com>
             David Ung <davidu@mips.com>

	* gas/mips/24k-branch-delay-1.d: New.
	* gas/mips/24k-branch-delay-1.s: New.
	* gas/mips/24k-triple-stores-1.d: New.
	* gas/mips/24k-triple-stores-1.s: New.
	* gas/mips/24k-triple-stores-2.d: New.
	* gas/mips/24k-triple-stores-2.s: New.
	* gas/mips/24k-triple-stores-3.d: New.
	* gas/mips/24k-triple-stores-3.s: New.
	* gas/mips/24k-triple-stores-4.s: New.
	* gas/mips/24k-triple-stores-4.d: New.
	* gas/mips/24k-triple-stores-5.d: New.
	* gas/mips/24k-triple-stores-5.s: New.
	* gas/mips/24k-triple-stores-6.d: New.
	* gas/mips/24k-triple-stores-6.s: New.
	* gas/mips/24k-triple-stores-7.d: New.
	* gas/mips/24k-triple-stores-7.s: New.
	* gas/mips/24k-triple-stores-8.d: New.
	* gas/mips/24k-triple-stores-8.s: New.
	* gas/mips/24k-triple-stores-9.d: New.
	* gas/mips/24k-triple-stores-9.s: New.
	* gas/mips/24k-triple-stores-10.d: New.
	* gas/mips/24k-triple-stores-10.s: New.
	* gas/mips/24k-triple-stores-11.d: New.
	* gas/mips/24k-triple-stores-11.s: New.
	* gas/mips/mips.exp: Invoke new tests.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 24k.patch-rev3
URL: <https://sourceware.org/pipermail/binutils/attachments/20110420/08df183f/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 24k-testsuite.patch-rev3
URL: <https://sourceware.org/pipermail/binutils/attachments/20110420/08df183f/attachment-0001.ksh>