[patch, spu] Fix GTY bugs with SPU builtins

trevor_smigiel@playstation.sony.com trevor_smigiel@playstation.sony.com
Thu Apr 23 00:16:00 GMT 2009


Hi Uli,

Yes, this patch is ok for mainline, 4.4 and 4.3.

Trevor

* Ulrich Weigand <uweigand@de.ibm.com> [2009-04-22 16:25]:
> Hello,
> 
> the spu_builtins and spu_builtin_types data structures are not properly
> registered with the garbage collection (GTY) mechanism.  This causes the
> following test suite failures for me:
> 
> FAIL: gcc.dg/pch/static-3.c  -O3 -fomit-frame-pointer  -I. (internal compiler error)
> FAIL: gcc.dg/pch/static-3.c  -O3 -fomit-frame-pointer  -I. (test for excess errors)
> FAIL: gcc.dg/pch/static-3.c  -O3 -fomit-frame-pointer  assembly comparison
> FAIL: gcc.dg/pch/static-3.c  -O3 -g  -I. (internal compiler error)
> FAIL: gcc.dg/pch/static-3.c  -O3 -g  -I. (test for excess errors)
> FAIL: gcc.dg/pch/static-3.c  -O3 -g  assembly comparison
> 
> and can most likely cause random crashes during garbage collection.
> 
> The problem is somewhat difficult to fix.  The structures are defined in
> the file spu-builtins.h, but this file is never parsed by the GT machinery;
> only the master header, spu.h, is.
> 
> To fix this, we could try to add the contents of spu-builtins.h to spu.h.
> However, due to the tricks being played with including spu-builtins.def,
> we probably do not want to do that (this would introduce a new dependency
> on spu-builtins.def on most files in GCC, which cannot easily be represented
> to the build system).
> 
> Therefore, this patch only moves enum spu_builtin_type,
> struct spu_builtin_description, and a generic declaration for spu_builtins
> to spu.h.  The type enum spu_builtin_type_index with its associated defines
> was solely used in spu.c anyway, so those are moved there.
> 
> The definition of enum spu_function_code is a bit of a problem.  Due to the
> spu-builtins.def include tricks, I've had to move it to spu.c as well.
> However, the type was also used in struct spu_builtin_description,
> and some of the enum values in spu-c.c.  I've now replaced the uses of the
> type with a plain int.
> 
> Instead of using explicit enum values to enumerate builtins all of whose
> arguments are of scalar type, I've changed spu_resolve_overloaded_builtin
> to detect this property programmatically (which seems to be preferable
> anyway, in case that list ever changes in the future).
> 
> With those changes, I'm now able to fully register those variables as
> proper roots with the garbage collector.  This fixes the above failures.
> 
> Tested on spu-elf with no regressions.
> 
> OK for mainline, 4.4 and 4.3 branches?
> 
> Bye,
> Ulrich
> 
> 
> ChangeLog:
> 
> 	* config/spu/spu-builtins.h: Delete file.
> 
> 	* config/spu/spu.h (enum spu_builtin_type): Move here from
> 	spu-builtins.h.
> 	(struct spu_builtin_description): Likewise.  Add GTY marker.
> 	Do not use enum spu_function_code or enum insn_code.
> 	(spu_builtins): Add extern declaration.
> 
> 	* config/spu/spu.c: Do not include "spu-builtins.h".
> 	(enum spu_function_code, enum spu_builtin_type_index,
> 	V16QI_type_node, V8HI_type_node, V4SI_type_node, V2DI_type_node,
> 	V4SF_type_node, V2DF_type_node, unsigned_V16QI_type_node,
> 	unsigned_V8HI_type_node, unsigned_V4SI_type_node,
> 	unsigned_V2DI_type_node): Move here from spu-builtins.h.
> 	(spu_builtin_types): Make static.  Add GTY marker.
> 	(spu_builtins): Add extern declaration with GTY marker.
> 	Include "gt-spu.h".
> 
> 	* config/spu/spu-c.c: Do not include "spu-builtins.h".
> 	(spu_resolve_overloaded_builtin): Do not use spu_function_code.
> 	Check programmatically whether all parameters are scalar.
> 
> 	* config/spu/t-spu-elf (spu.o, spu-c.o): Update dependencies.



More information about the Gcc-patches mailing list