[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