This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch, spu] Fix GTY bugs with SPU builtins


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.


Index: gcc/config/spu/spu-c.c
===================================================================
*** gcc/config/spu/spu-c.c	(revision 146574)
--- gcc/config/spu/spu-c.c	(working copy)
***************
*** 32,38 ****
  #include "insn-codes.h"
  #include "recog.h"
  #include "optabs.h"
- #include "spu-builtins.h"
  
  
  /* Keep the vector keywords handy for fast comparisons.  */
--- 32,37 ----
*************** spu_resolve_overloaded_builtin (tree fnd
*** 103,110 ****
  			  || POINTER_TYPE_P (t))
    VEC(tree,gc) *fnargs = (VEC(tree,gc) *) passed_args;
    unsigned int nargs = VEC_length (tree, fnargs);
!   spu_function_code new_fcode, fcode =
!     DECL_FUNCTION_CODE (fndecl) - END_BUILTINS;
    struct spu_builtin_description *desc;
    tree match = NULL_TREE;
  
--- 102,108 ----
  			  || POINTER_TYPE_P (t))
    VEC(tree,gc) *fnargs = (VEC(tree,gc) *) passed_args;
    unsigned int nargs = VEC_length (tree, fnargs);
!   int new_fcode, fcode = DECL_FUNCTION_CODE (fndecl) - END_BUILTINS;
    struct spu_builtin_description *desc;
    tree match = NULL_TREE;
  
*************** spu_resolve_overloaded_builtin (tree fnd
*** 124,131 ****
--- 122,136 ----
        tree decl = spu_builtins[new_fcode].fndecl;
        tree params = TYPE_ARG_TYPES (TREE_TYPE (decl));
        tree param;
+       bool all_scalar;
        unsigned int p;
  
+       /* Check whether all parameters are scalar.  */
+       all_scalar = true;
+       for (param = params; param != void_list_node; param = TREE_CHAIN (param))
+       if (!SCALAR_TYPE_P (TREE_VALUE (param)))
+ 	all_scalar = false;
+ 
        for (param = params, p = 0;
  	   param != void_list_node;
  	   param = TREE_CHAIN (param), p++)
*************** spu_resolve_overloaded_builtin (tree fnd
*** 157,166 ****
  	     parameter. */
  	  if ((!SCALAR_TYPE_P (param_type)
  	       || !SCALAR_TYPE_P (arg_type)
! 	       || ((fcode == SPU_SPLATS || fcode == SPU_PROMOTE
! 		    || fcode == SPU_HCMPEQ || fcode == SPU_HCMPGT
! 		    || fcode == SPU_MASKB || fcode == SPU_MASKH
! 		    || fcode == SPU_MASKW) && p == 0))
  	      && !comptypes (TYPE_MAIN_VARIANT (param_type),
  			     TYPE_MAIN_VARIANT (arg_type)))
  	    break;
--- 162,168 ----
  	     parameter. */
  	  if ((!SCALAR_TYPE_P (param_type)
  	       || !SCALAR_TYPE_P (arg_type)
! 	       || (all_scalar && p == 0))
  	      && !comptypes (TYPE_MAIN_VARIANT (param_type),
  			     TYPE_MAIN_VARIANT (arg_type)))
  	    break;
Index: gcc/config/spu/t-spu-elf
===================================================================
*** gcc/config/spu/t-spu-elf	(revision 146574)
--- gcc/config/spu/t-spu-elf	(working copy)
*************** spu.o: $(CONFIG_H) $(SYSTEM_H) coretypes
*** 85,97 ****
    output.h $(BASIC_BLOCK_H) $(INTEGRATE_H) toplev.h $(GGC_H) $(HASHTAB_H) \
    $(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h cfglayout.h \
    $(srcdir)/config/spu/spu-protos.h \
-   $(srcdir)/config/spu/spu-builtins.h \
    $(srcdir)/config/spu/spu-builtins.def 
  
  spu-c.o: $(srcdir)/config/spu/spu-c.c \
      $(srcdir)/config/spu/spu-protos.h \
-     $(srcdir)/config/spu/spu-builtins.h \
-     $(srcdir)/config/spu/spu-builtins.def \
      $(CONFIG_H) $(SYSTEM_H) $(TREE_H) $(CPPLIB_H) \
      $(TM_P_H) c-pragma.h errors.h coretypes.h $(TM_H) insn-codes.h
  	$(CC) -c $(ALL_CFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) $(srcdir)/config/spu/spu-c.c
--- 85,94 ----
Index: gcc/config/spu/spu.c
===================================================================
*** gcc/config/spu/spu.c	(revision 146574)
--- gcc/config/spu/spu.c	(working copy)
***************
*** 52,64 ****
  #include "machmode.h"
  #include "gimple.h"
  #include "tm-constrs.h"
- #include "spu-builtins.h"
  #include "ddg.h"
  #include "sbitmap.h"
  #include "timevar.h"
  #include "df.h"
  
  /* Builtin types, data and prototypes. */
  struct spu_builtin_range
  {
    int low, high;
--- 52,130 ----
  #include "machmode.h"
  #include "gimple.h"
  #include "tm-constrs.h"
  #include "ddg.h"
  #include "sbitmap.h"
  #include "timevar.h"
  #include "df.h"
  
  /* Builtin types, data and prototypes. */
+ 
+ enum spu_builtin_type_index
+ {
+   SPU_BTI_END_OF_PARAMS,
+ 
+   /* We create new type nodes for these. */
+   SPU_BTI_V16QI,
+   SPU_BTI_V8HI,
+   SPU_BTI_V4SI,
+   SPU_BTI_V2DI,
+   SPU_BTI_V4SF,
+   SPU_BTI_V2DF,
+   SPU_BTI_UV16QI,
+   SPU_BTI_UV8HI,
+   SPU_BTI_UV4SI,
+   SPU_BTI_UV2DI,
+ 
+   /* A 16-byte type. (Implemented with V16QI_type_node) */
+   SPU_BTI_QUADWORD,
+ 
+   /* These all correspond to intSI_type_node */
+   SPU_BTI_7,
+   SPU_BTI_S7,
+   SPU_BTI_U7,
+   SPU_BTI_S10,
+   SPU_BTI_S10_4,
+   SPU_BTI_U14,
+   SPU_BTI_16,
+   SPU_BTI_S16,
+   SPU_BTI_S16_2,
+   SPU_BTI_U16,
+   SPU_BTI_U16_2,
+   SPU_BTI_U18,
+ 
+   /* These correspond to the standard types */
+   SPU_BTI_INTQI, 
+   SPU_BTI_INTHI, 
+   SPU_BTI_INTSI, 
+   SPU_BTI_INTDI, 
+ 
+   SPU_BTI_UINTQI,
+   SPU_BTI_UINTHI,
+   SPU_BTI_UINTSI,
+   SPU_BTI_UINTDI,
+ 
+   SPU_BTI_FLOAT, 
+   SPU_BTI_DOUBLE,
+ 
+   SPU_BTI_VOID,   
+   SPU_BTI_PTR,   
+ 
+   SPU_BTI_MAX
+ };
+ 
+ #define V16QI_type_node               (spu_builtin_types[SPU_BTI_V16QI])
+ #define V8HI_type_node                (spu_builtin_types[SPU_BTI_V8HI])
+ #define V4SI_type_node                (spu_builtin_types[SPU_BTI_V4SI])
+ #define V2DI_type_node                (spu_builtin_types[SPU_BTI_V2DI])
+ #define V4SF_type_node                (spu_builtin_types[SPU_BTI_V4SF])
+ #define V2DF_type_node                (spu_builtin_types[SPU_BTI_V2DF])
+ #define unsigned_V16QI_type_node      (spu_builtin_types[SPU_BTI_UV16QI])
+ #define unsigned_V8HI_type_node       (spu_builtin_types[SPU_BTI_UV8HI])
+ #define unsigned_V4SI_type_node       (spu_builtin_types[SPU_BTI_UV4SI])
+ #define unsigned_V2DI_type_node       (spu_builtin_types[SPU_BTI_UV2DI])
+ 
+ static GTY(()) tree spu_builtin_types[SPU_BTI_MAX];
+ 
  struct spu_builtin_range
  {
    int low, high;
*************** spu_libgcc_cmp_return_mode (void);
*** 202,209 ****
  static enum machine_mode
  spu_libgcc_shift_count_mode (void);
  
- /* Built in types.  */
- tree spu_builtin_types[SPU_BTI_MAX];
  
  /*  TARGET overrides.  */
  
--- 268,273 ----
*************** spu_return_in_memory (const_tree type, c
*** 5059,5064 ****
--- 5123,5138 ----
  
  /* Create the built-in types and functions */
  
+ enum spu_function_code
+ {
+ #define DEF_BUILTIN(fcode, icode, name, type, params) fcode,
+ #include "spu-builtins.def"
+ #undef DEF_BUILTIN
+    NUM_SPU_BUILTINS
+ };
+ 
+ extern GTY(()) struct spu_builtin_description spu_builtins[NUM_SPU_BUILTINS];
+ 
  struct spu_builtin_description spu_builtins[] = {
  #define DEF_BUILTIN(fcode, icode, name, type, params) \
    {fcode, icode, name, type, params, NULL_TREE},
*************** spu_section_type_flags (tree decl, const
*** 6289,6291 ****
--- 6363,6367 ----
      return SECTION_BSS;
    return default_section_type_flags (decl, name, reloc);
  }
+ 
+ #include "gt-spu.h"
Index: gcc/config/spu/spu.h
===================================================================
*** gcc/config/spu/spu.h	(revision 146574)
--- gcc/config/spu/spu.h	(working copy)
*************** targetm.resolve_overloaded_builtin = spu
*** 622,624 ****
--- 622,654 ----
  extern GTY(()) rtx spu_compare_op0;
  extern GTY(()) rtx spu_compare_op1;
  
+ 
+ /* Builtins.  */
+ 
+ enum spu_builtin_type
+ {
+   B_INSN,
+   B_JUMP,
+   B_BISLED,
+   B_CALL,
+   B_HINT,
+   B_OVERLOAD,
+   B_INTERNAL
+ };
+ 
+ struct spu_builtin_description GTY(())
+ {
+   int fcode;
+   int icode;
+   const char *name;
+   enum spu_builtin_type type;
+ 
+   /* The first element of parm is always the return type.  The rest
+      are a zero terminated list of parameters.  */
+   int parm[5];
+ 
+   tree fndecl;
+ };
+ 
+ extern struct spu_builtin_description spu_builtins[];
+ 
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]