Patchwork rs6000: Extend TARGET_NO_LWSYNC to cover more processors

login
register
mail settings
Submitter Meador Inge
Date March 27, 2012, 9:21 p.m.
Message ID <4F722F72.8020405@codesourcery.com>
Download mbox | patch
Permalink /patch/149035/
State New
Headers show

Comments

Meador Inge - March 27, 2012, 9:21 p.m.
Hi All,

This patch fixes an issue reported by one of our customers where an instruction
exception gets raised when using '__sync_fetch_and_add' on a PowerPC 440
processor.  The instruction causing the exception is 'lwsync'.  Luckily Joseph
laid the groundwork when solving a similar issue for e500 cores [1] by adding a
new macro ('TARGET_NO_LWSYNC') for controlling whether 'lwsync' is available .

This patch extends the 'TARGET_NO_LWSYNC' macro to include the PowerPC 440
and 603 processors.  The 440 because that is what the problem was reported
against and the 603 because problems have been reported elsewhere [4] about
that.  It doesn't seem like 'lwsync' is supported on 603 processors anyway.  I
looked at the IBM [2] and Freescale [3] manuals and both use the heavyweight
implementation of 'sync' (i.e. the 'sync' bit L=0).

FWIW, I also took a look at the Linux kernel code and 'lwsync' is only used
on 64-bit PowerPC processors and e500 processors that can support it.  This
can be seen in 'arch/powerpc/include/asm/synch.h':

   #if defined(__powerpc64__)
   #    define LWSYNC	lwsync
   #elif defined(CONFIG_E500)
   #    define LWSYNC					\
	START_LWSYNC_SECTION(96);			\
	sync;						\
	MAKE_LWSYNC_SECTION_ENTRY(96, __lwsync_fixup);
   #else
   #    define LWSYNC	sync
   #endif

Support for the e500 processors is determined at runtime and the kernel is
dynamically patched.

Regression tested with powerpc-none-eabi.

OK?

P.S.  If it is OK can some please commit for me?  I don't have write access.

[1] http://gcc.gnu.org/ml/gcc-patches/2006-11/msg01238.html
[2]
https://www-01.ibm.com/chips/techlib/techlib.nsf/techdocs/852569B20050FF7785256996006F9795/$file/603e_um.pdf
[3] http://cache.freescale.com/files/32bit/doc/ref_manual/MPC603EUM.pdf
[4] http://gcc.gnu.org/ml/gcc/2008-06/msg00449.html
David Edelsohn - March 28, 2012, 8:59 p.m.
On Tue, Mar 27, 2012 at 5:21 PM, Meador Inge <meadori@codesourcery.com> wrote:
> Hi All,
>
> This patch fixes an issue reported by one of our customers where an instruction
> exception gets raised when using '__sync_fetch_and_add' on a PowerPC 440
> processor.  The instruction causing the exception is 'lwsync'.  Luckily Joseph
> laid the groundwork when solving a similar issue for e500 cores [1] by adding a
> new macro ('TARGET_NO_LWSYNC') for controlling whether 'lwsync' is available .
>
> This patch extends the 'TARGET_NO_LWSYNC' macro to include the PowerPC 440
> and 603 processors.  The 440 because that is what the problem was reported
> against and the 603 because problems have been reported elsewhere [4] about
> that.  It doesn't seem like 'lwsync' is supported on 603 processors anyway.  I
> looked at the IBM [2] and Freescale [3] manuals and both use the heavyweight
> implementation of 'sync' (i.e. the 'sync' bit L=0).

Meador,

Something does not make sense about this patch.  Other than unique
issues with e500, lwsync should be accepted everywhere.  On older
processors, the L bit is ignored and it is treated as hwsync.  So I do
not understand the need for explicit TARGET_NO_LWSYNC on PPC440 or
PPC603.

Is this some sort of PPC440 errata for the specific 440 being used by
Mentor's customer?

Thanks, David
Meador Inge - March 28, 2012, 9:32 p.m.
On 03/28/2012 03:59 PM, David Edelsohn wrote:

> On Tue, Mar 27, 2012 at 5:21 PM, Meador Inge <meadori@codesourcery.com> wrote:
>> Hi All,
>>
>> This patch fixes an issue reported by one of our customers where an instruction
>> exception gets raised when using '__sync_fetch_and_add' on a PowerPC 440
>> processor.  The instruction causing the exception is 'lwsync'.  Luckily Joseph
>> laid the groundwork when solving a similar issue for e500 cores [1] by adding a
>> new macro ('TARGET_NO_LWSYNC') for controlling whether 'lwsync' is available .
>>
>> This patch extends the 'TARGET_NO_LWSYNC' macro to include the PowerPC 440
>> and 603 processors.  The 440 because that is what the problem was reported
>> against and the 603 because problems have been reported elsewhere [4] about
>> that.  It doesn't seem like 'lwsync' is supported on 603 processors anyway.  I
>> looked at the IBM [2] and Freescale [3] manuals and both use the heavyweight
>> implementation of 'sync' (i.e. the 'sync' bit L=0).
> 
> Meador,
> 
> Something does not make sense about this patch.  Other than unique
> issues with e500, lwsync should be accepted everywhere.  On older
> processors, the L bit is ignored and it is treated as hwsync.  So I do
> not understand the need for explicit TARGET_NO_LWSYNC on PPC440 or
> PPC603.
> 
> Is this some sort of PPC440 errata for the specific 440 being used by
> Mentor's customer?

David,

I am still working on getting the specific processor information.  Thanks
for the lwsync info and review feedback.  If I can't get the processor
specifics, then I will just drop the patch.

Thanks again,

Patch

Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 185881)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -501,8 +501,10 @@  extern int rs6000_vector_align[];
 
 
 
-/* E500 processors only support plain "sync", not lwsync.  */
-#define TARGET_NO_LWSYNC TARGET_E500
+/* Some processors only support plain "sync", not lwsync.  */
+#define TARGET_NO_LWSYNC (TARGET_E500 \
+			  || rs6000_cpu == PROCESSOR_PPC440 \
+			  || rs6000_cpu == PROCESSOR_PPC603)
 
 /* Which machine supports the various reciprocal estimate instructions.  */
 #define TARGET_FRES	(TARGET_HARD_FLOAT && TARGET_PPC_GFXOPT \