This is the mail archive of the
mailing list for the GCC project.
Re: [patch, spu] Fix GTY bugs with SPU builtins
- From: trevor_smigiel at playstation dot sony dot com
- To: Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: gcc-patches at gcc dot gnu dot org, andrew_pinski at playstation dot sony dot com
- Date: Wed, 22 Apr 2009 16:39:08 -0700
- Subject: Re: [patch, spu] Fix GTY bugs with SPU builtins
- References: <200904222316.n3MNGME5008394@d12av02.megacenter.de.ibm.com>
Yes, this patch is ok for mainline, 4.4 and 4.3.
* Ulrich Weigand <firstname.lastname@example.org> [2009-04-22 16:25]:
> 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?
> * config/spu/spu-builtins.h: Delete file.
> * config/spu/spu.h (enum spu_builtin_type): Move here from
> (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.