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]

[trans-mem] rms-tm bug report

From: Aldy Hernandez <aldyh at redhat dot com>
To: Patrick Marlier <patrick dot marlier at unine dot ch>
Cc: Richard Henderson <rth at redhat dot com>, FELBER Pascal <pascal dot felber at unine dot ch>, Javier Arias <javier dot arias at bsc dot es>, "gokcen dot kestor at bsc dot es" <gokcen dot kestor at bsc dot es>, gcc-patches at gcc dot gnu dot org
Date: Mon, 24 May 2010 13:18:13 -0400
Subject: [trans-mem] rms-tm bug report
References: <4BF4DD38.3080708@unine.ch> <20100524171655.GA19900@redhat.com>

I'm an idiot.  I keep forgetting to CC the list.

On Mon, May 24, 2010 at 01:16:55PM -0400, Aldy Hernandez wrote:
> Here is another bug report from Patrick extracted from the RMS-TM
> benchmark.  No worries Patrick; keep 'em coming. :)
> 
> The problem here is that the vectorizer creates an 128-bit load (V2DI)
> of a vector constant (V = {0, 0}).  Since any unhandled loads/stores are
> generated as a BUILT_IN_TM_MEMMOVE, we generate an invalid memmove while
> trying to get the address of a VECTOR_CST.
> 
> The TM abi provides 128/256 bit TM loads/stores, so it's just a matter
> of generating them, since the library already handles them.
> 
> I am assuming that an architecture that has 128 bit vectors, has at
> least V2DI or V4SI instructions, similarly for 256 bit vectors
> (V4DI/V8SI).
> 
> OK for branch?
> 
> 	* builtin-types.def (BT_I32): New.
> 	(BT_FN_I16_VPTR): New.
> 	(BT_FN_I32_VPTR): New.
> 	(BT_FN_VOID_VPTR_I16): New.
> 	(BT_FN_VOID_VPTR_I32): New.
> 	* trans-mem.c: Include expr.h and optabs.h.
> 	(is_tm_simple_load): Handle 128 and 256 bit variants.
> 	(is_tm_simple_store): Same.
> 	(build_tm_load): Same.
> 	(build_tm_store): Same.
> 	(gtm-builtins.def): Define BUILT_IN_TM_STORE*_16 variants.
> 	Define BUILT_IN_TM_STORE*_32 variants.
> 	Define BUILT_IN_TM_LOAD*_16 variants.
> 	Define BUILT_IN_TM_LOAD*_32 variants.
> 	(mode_available_p): New.
> 	* Makefile.in (trans-mem.o): Depend on EXPR_H and OPTABS_H.
> 
> Index: testsuite/g++.dg/tm/20100524.c
> ===================================================================
> --- testsuite/g++.dg/tm/20100524.c	(revision 0)
> +++ testsuite/g++.dg/tm/20100524.c	(revision 0)
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fgnu-tm -O3" } */
> +
> +class HashTree
> +{  
> +   __attribute__((transaction_safe)) void rehash();
> +   HashTree **Hash_table;
> +   int Hash_function;
> +};
> +
> +__attribute__((transaction_safe)) void HashTree::rehash()
> +{  
> +   for (int i=0; i < Hash_function; i++)
> +      Hash_table[i] = 0;
> +}
> Index: builtin-types.def
> ===================================================================
> --- builtin-types.def	(revision 158824)
> +++ builtin-types.def	(working copy)
> @@ -120,6 +120,7 @@ DEF_PRIMITIVE_TYPE (BT_I2, builtin_type_
>  DEF_PRIMITIVE_TYPE (BT_I4, builtin_type_for_size (BITS_PER_UNIT*4, 1))
>  DEF_PRIMITIVE_TYPE (BT_I8, builtin_type_for_size (BITS_PER_UNIT*8, 1))
>  DEF_PRIMITIVE_TYPE (BT_I16, builtin_type_for_size (BITS_PER_UNIT*16, 1))
> +DEF_PRIMITIVE_TYPE (BT_I32, builtin_type_for_size (BITS_PER_UNIT*32, 1))
>  
>  DEF_POINTER_TYPE (BT_PTR_CONST_STRING, BT_CONST_STRING)
>  DEF_POINTER_TYPE (BT_PTR_LONG, BT_LONG)
> @@ -479,6 +480,8 @@ DEF_FUNCTION_TYPE_1 (BT_FN_I1_VPTR, BT_I
>  DEF_FUNCTION_TYPE_1 (BT_FN_I2_VPTR, BT_I2, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_I4_VPTR, BT_I4, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_I8_VPTR, BT_I8, BT_VOLATILE_PTR)
> +DEF_FUNCTION_TYPE_1 (BT_FN_I16_VPTR, BT_I16, BT_VOLATILE_PTR)
> +DEF_FUNCTION_TYPE_1 (BT_FN_I32_VPTR, BT_I32, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_FLOAT_VPTR, BT_FLOAT, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_DOUBLE_VPTR, BT_DOUBLE, BT_VOLATILE_PTR)
>  DEF_FUNCTION_TYPE_1 (BT_FN_LDOUBLE_VPTR, BT_LONGDOUBLE, BT_VOLATILE_PTR)
> @@ -487,6 +490,8 @@ DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I1,
>  DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I2, BT_VOID, BT_VOLATILE_PTR, BT_I2)
>  DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I4, BT_VOID, BT_VOLATILE_PTR, BT_I4)
>  DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I8, BT_VOID, BT_VOLATILE_PTR, BT_I8)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I16, BT_VOID, BT_VOLATILE_PTR, BT_I16)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_I32, BT_VOID, BT_VOLATILE_PTR, BT_I32)
>  DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_FLOAT, BT_VOID, BT_VOLATILE_PTR, BT_FLOAT)
>  DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_DOUBLE, BT_VOID, 
>                       BT_VOLATILE_PTR, BT_DOUBLE)
> Index: trans-mem.c
> ===================================================================
> --- trans-mem.c	(revision 159601)
> +++ trans-mem.c	(working copy)
> @@ -38,6 +38,8 @@
>  #include "params.h"
>  #include "target.h"
>  #include "langhooks.h"
> +#include "expr.h"
> +#include "optabs.h"
>  
>  
>  #define PROB_VERY_UNLIKELY	(REG_BR_PROB_BASE / 2000 - 1)
> @@ -344,6 +346,8 @@ is_tm_simple_load (gimple stmt)
>  	      || fcode == BUILT_IN_TM_LOAD_2
>  	      || fcode == BUILT_IN_TM_LOAD_4
>  	      || fcode == BUILT_IN_TM_LOAD_8
> +	      || fcode == BUILT_IN_TM_LOAD_16
> +	      || fcode == BUILT_IN_TM_LOAD_32
>  	      || fcode == BUILT_IN_TM_LOAD_FLOAT
>  	      || fcode == BUILT_IN_TM_LOAD_DOUBLE
>  	      || fcode == BUILT_IN_TM_LOAD_LDOUBLE);
> @@ -385,6 +389,8 @@ is_tm_simple_store (gimple stmt)
>  	      || fcode == BUILT_IN_TM_STORE_2
>  	      || fcode == BUILT_IN_TM_STORE_4
>  	      || fcode == BUILT_IN_TM_STORE_8
> +	      || fcode == BUILT_IN_TM_STORE_16
> +	      || fcode == BUILT_IN_TM_STORE_32
>  	      || fcode == BUILT_IN_TM_STORE_FLOAT
>  	      || fcode == BUILT_IN_TM_STORE_DOUBLE
>  	      || fcode == BUILT_IN_TM_STORE_LDOUBLE);
> @@ -1860,6 +1866,15 @@ transaction_subcode_ior (struct tm_regio
>      }
>  }
>  
> +/* Return TRUE if the backend has a mov operation for the given
> +   MODE.  */
> +
> +static inline bool
> +mode_available_p (enum machine_mode mode)
> +{
> +  return (optab_handler (mov_optab, mode)->insn_code != CODE_FOR_nothing);
> +}
> +
>  /* Construct a memory load in a transactional context.  Return the
>     gimple statement performing the load, or NULL if there is no
>     TM_LOAD builtin of the appropriate size to do the load.
> @@ -1896,6 +1911,16 @@ build_tm_load (location_t loc, tree lhs,
>  	case 8:
>  	  code = BUILT_IN_TM_LOAD_8;
>  	  break;
> +	case 16:
> +	  if (mode_available_p (V2DImode)
> +	      || mode_available_p (V4SImode))
> +	    code = BUILT_IN_TM_LOAD_16;
> +	  break;
> +	case 32:
> +	  if (mode_available_p (V4DImode)
> +	      || mode_available_p (V8SImode))
> +	    code = BUILT_IN_TM_LOAD_32;
> +	  break;
>  	}
>      }
>  
> @@ -1962,6 +1987,16 @@ build_tm_store (location_t loc, tree lhs
>  	case 8:
>  	  code = BUILT_IN_TM_STORE_8;
>  	  break;
> +	case 16:
> +	  if (mode_available_p (V2DImode)
> +	      || mode_available_p (V4SImode))
> +	    code = BUILT_IN_TM_STORE_16;
> +	  break;
> +	case 32:
> +	  if (mode_available_p (V4DImode)
> +	      || mode_available_p (V8SImode))
> +	    code = BUILT_IN_TM_STORE_32;
> +	  break;
>  	}
>      }
>  
> Index: gtm-builtins.def
> ===================================================================
> --- gtm-builtins.def	(revision 159603)
> +++ gtm-builtins.def	(working copy)
> @@ -81,6 +81,20 @@ DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAR_8,
>  DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAW_8, "_ITM_WaWU8",
>  		BT_FN_VOID_VPTR_I8, ATTR_TM_NOTHROW_LIST)
>  
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_16, "_ITM_WM128",
> +		BT_FN_VOID_VPTR_I16, ATTR_TM_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAR_16, "_ITM_WaRM128",
> +		BT_FN_VOID_VPTR_I16, ATTR_TM_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAW_16, "_ITM_WaWM128",
> +		BT_FN_VOID_VPTR_I16, ATTR_TM_NOTHROW_LIST)
> +
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_32, "_ITM_WM256",
> +		BT_FN_VOID_VPTR_I32, ATTR_TM_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAR_32, "_ITM_WaRM256",
> +		BT_FN_VOID_VPTR_I32, ATTR_TM_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAW_32, "_ITM_WaWM256",
> +		BT_FN_VOID_VPTR_I32, ATTR_TM_NOTHROW_LIST)
> +
>  DEF_TM_BUILTIN (BUILT_IN_TM_STORE_FLOAT, "_ITM_WF",
>  		BT_FN_VOID_VPTR_FLOAT, ATTR_TM_NOTHROW_LIST)
>  DEF_TM_BUILTIN (BUILT_IN_TM_STORE_WAR_FLOAT, "_ITM_WaRF",
> @@ -144,6 +158,24 @@ DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAW_8, 
>  DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RFW_8, "_ITM_RfWU8",
>  		BT_FN_I8_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
>  
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_16, "_ITM_RM128",
> +		BT_FN_I16_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAR_16, "_ITM_RaRM128",
> +		BT_FN_I16_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAW_16, "_ITM_RaWM128",
> +		BT_FN_I16_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RFW_16, "_ITM_RfWM128",
> +		BT_FN_I16_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_32, "_ITM_RM256",
> +		BT_FN_I32_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAR_32, "_ITM_RaRM256",
> +		BT_FN_I32_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAW_32, "_ITM_RaWM256",
> +		BT_FN_I32_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RFW_32, "_ITM_RfWM256",
> +		BT_FN_I32_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
> +
>  DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_FLOAT, "_ITM_RF",
>  		BT_FN_FLOAT_VPTR, ATTR_TM_PURE_TMPURE_NOTHROW_LIST)
>  DEF_TM_BUILTIN (BUILT_IN_TM_LOAD_RAR_FLOAT, "_ITM_RaRF",
> Index: Makefile.in
> ===================================================================
> --- Makefile.in	(revision 158829)
> +++ Makefile.in	(working copy)
> @@ -2187,7 +2187,7 @@ gtype-desc.o: gtype-desc.c $(CONFIG_H) $
>  trans-mem.o : trans-mem.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
>  	$(TREE_H) $(GIMPLE_H) $(TREE_FLOW_H) tree-pass.h except.h \
>  	$(DIAGNOSTIC_H) $(TOPLEV_H) $(FLAGS_H) $(DEMANGLE_H) $(TRANS_MEM_H) \
> -	$(TREE_DUMP_H) $(PARAMS_H)
> +	$(TREE_DUMP_H) $(PARAMS_H) $(EXPR_H) $(OPTABS_H)
>  
>  ggc-common.o: ggc-common.c $(CONFIG_H) $(SYSTEM_H) coretypes.h		\
>  	$(GGC_H) $(HASHTAB_H) $(TOPLEV_H) $(PARAMS_H) hosthooks.h	\

Follow-Ups:
- Re: [trans-mem] rms-tm bug report
  - From: Steven Bosscher
- Re: [trans-mem] rms-tm bug report
  - From: Richard Henderson

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