[PATCH][C++] Kill expr_size langhook

[PATCH][C++] Kill expr_size langhook

[Richard Guenther]

This kills the expr_size langhook.  With the recent C++ FE change
there isn't any language-specific semantics left that warrant it.
In particular the C++ FE is already careful enough to use memcpy
whenever an assignment may cover padding that is used by derived
classes.

Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?

Thanks,
Richard.

2009-08-29  Richard Guenther  <rguenther@suse.de>

	* tree.c (tree_expr_size): New function.
	* tree.h (tree_expr_size): Declare.
	* rtlanal.c (rtx_addr_can_trap_p_1): Adjust comment.
	* builtins.c (fold_builtin_memory_op): Use tree_expr_size.
	* langhooks.c (lhd_expr_size): Remove.
	* langhooks.h (struct lang_hooks): Remove expr_size.
	* explow.c (expr_size): Use tree_expr_size.
	(int_expr_size): Likewise.
	* langhooks-def.h (lhd_expr_size): Remove.
	(LANG_HOOKS_EXPR_SIZE): Likewise.
	(LANG_HOOKS_INITIALIZER): Adjust.

	cp/
	* cp-objcp-common.c (cp_expr_size): Use tree_expr_size.
	* cp-objcp-common.h (LANG_HOOKS_EXPR_SIZE): Do not define.

Index: gcc/tree.c
===================================================================
*** gcc/tree.c	(revision 151175)
--- gcc/tree.c	(working copy)
*************** max_int_size_in_bytes (const_tree type)
*** 2194,2199 ****
--- 2194,2211 ----
  
    return size;
  }
+ 
+ /* Returns a tree for the size of EXP in bytes.  */
+ 
+ tree
+ tree_expr_size (const_tree exp)
+ {
+   if (DECL_P (exp)
+       && DECL_SIZE_UNIT (exp) != 0)
+     return DECL_SIZE_UNIT (exp);
+   else
+     return size_in_bytes (TREE_TYPE (exp));
+ }
  \f
  /* Return the bit position of FIELD, in bits from the start of the record.
     This is a tree of type bitsizetype.  */
Index: gcc/tree.h
===================================================================
*** gcc/tree.h	(revision 151175)
--- gcc/tree.h	(working copy)
*************** extern tree expr_last (tree);
*** 4180,4185 ****
--- 4180,4186 ----
  extern tree size_in_bytes (const_tree);
  extern HOST_WIDE_INT int_size_in_bytes (const_tree);
  extern HOST_WIDE_INT max_int_size_in_bytes (const_tree);
+ extern tree tree_expr_size (const_tree);
  extern tree bit_position (const_tree);
  extern HOST_WIDE_INT int_bit_position (const_tree);
  extern tree byte_position (const_tree);
Index: gcc/rtlanal.c
===================================================================
*** gcc/rtlanal.c	(revision 151175)
--- gcc/rtlanal.c	(working copy)
*************** rtx_addr_can_trap_p_1 (const_rtx x, HOST
*** 293,299 ****
  	  decl = SYMBOL_REF_DECL (x);
  
  	  /* Else check that the access is in bounds.  TODO: restructure
! 	     expr_size/lhd_expr_size/int_expr_size and just use the latter.  */
  	  if (!decl)
  	    decl_size = -1;
  	  else if (DECL_P (decl) && DECL_SIZE_UNIT (decl))
--- 293,299 ----
  	  decl = SYMBOL_REF_DECL (x);
  
  	  /* Else check that the access is in bounds.  TODO: restructure
! 	     expr_size/tree_expr_size/int_expr_size and just use the latter.  */
  	  if (!decl)
  	    decl_size = -1;
  	  else if (DECL_P (decl) && DECL_SIZE_UNIT (decl))
Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c	(revision 151175)
--- gcc/builtins.c	(working copy)
*************** fold_builtin_memory_op (location_t loc, 
*** 9136,9142 ****
  	  srcvar = build_fold_indirect_ref_loc (loc, src);
  	  if (TREE_THIS_VOLATILE (srcvar))
  	    return NULL_TREE;
! 	  else if (!tree_int_cst_equal (lang_hooks.expr_size (srcvar), len))
  	    srcvar = NULL_TREE;
  	  /* With memcpy, it is possible to bypass aliasing rules, so without
  	     this check i.e. execute/20060930-2.c would be misoptimized,
--- 9136,9142 ----
  	  srcvar = build_fold_indirect_ref_loc (loc, src);
  	  if (TREE_THIS_VOLATILE (srcvar))
  	    return NULL_TREE;
! 	  else if (!tree_int_cst_equal (tree_expr_size (srcvar), len))
  	    srcvar = NULL_TREE;
  	  /* With memcpy, it is possible to bypass aliasing rules, so without
  	     this check i.e. execute/20060930-2.c would be misoptimized,
*************** fold_builtin_memory_op (location_t loc, 
*** 9154,9160 ****
  	  destvar = build_fold_indirect_ref_loc (loc, dest);
  	  if (TREE_THIS_VOLATILE (destvar))
  	    return NULL_TREE;
! 	  else if (!tree_int_cst_equal (lang_hooks.expr_size (destvar), len))
  	    destvar = NULL_TREE;
  	  else if (!var_decl_component_p (destvar))
  	    destvar = NULL_TREE;
--- 9154,9160 ----
  	  destvar = build_fold_indirect_ref_loc (loc, dest);
  	  if (TREE_THIS_VOLATILE (destvar))
  	    return NULL_TREE;
! 	  else if (!tree_int_cst_equal (tree_expr_size (destvar), len))
  	    destvar = NULL_TREE;
  	  else if (!var_decl_component_p (destvar))
  	    destvar = NULL_TREE;
Index: gcc/cp/cp-objcp-common.c
===================================================================
*** gcc/cp/cp-objcp-common.c	(revision 151175)
--- gcc/cp/cp-objcp-common.c	(working copy)
*************** cp_expr_size (const_tree exp)
*** 109,115 ****
      }
    else
      /* Use the default code.  */
!     return lhd_expr_size (exp);
  }
  
  /* Langhook for tree_size: determine size of our 'x' and 'c' nodes.  */
--- 109,115 ----
      }
    else
      /* Use the default code.  */
!     return tree_expr_size (exp);
  }
  
  /* Langhook for tree_size: determine size of our 'x' and 'c' nodes.  */
Index: gcc/cp/cp-objcp-common.h
===================================================================
*** gcc/cp/cp-objcp-common.h	(revision 151175)
--- gcc/cp/cp-objcp-common.h	(working copy)
*************** extern bool cp_function_decl_explicit_p 
*** 101,108 ****
  #define LANG_HOOKS_TREE_DUMP_DUMP_TREE_FN cp_dump_tree
  #undef LANG_HOOKS_TREE_DUMP_TYPE_QUALS_FN
  #define LANG_HOOKS_TREE_DUMP_TYPE_QUALS_FN cp_type_quals
- #undef LANG_HOOKS_EXPR_SIZE
- #define LANG_HOOKS_EXPR_SIZE cp_expr_size
  
  #undef LANG_HOOKS_CALLGRAPH_ANALYZE_EXPR
  #define LANG_HOOKS_CALLGRAPH_ANALYZE_EXPR cxx_callgraph_analyze_expr
--- 101,106 ----
Index: gcc/langhooks.c
===================================================================
*** gcc/langhooks.c	(revision 151175)
--- gcc/langhooks.c	(working copy)
*************** lhd_tree_dump_type_quals (const_tree t)
*** 260,278 ****
    return TYPE_QUALS (t);
  }
  
- /* lang_hooks.expr_size: Determine the size of the value of an expression T
-    in a language-specific way.  Returns a tree for the size in bytes.  */
- 
- tree
- lhd_expr_size (const_tree exp)
- {
-   if (DECL_P (exp)
-       && DECL_SIZE_UNIT (exp) != 0)
-     return DECL_SIZE_UNIT (exp);
-   else
-     return size_in_bytes (TREE_TYPE (exp));
- }
- 
  /* lang_hooks.gimplify_expr re-writes *EXPR_P into GIMPLE form.  */
  
  int
--- 260,265 ----
Index: gcc/langhooks.h
===================================================================
*** gcc/langhooks.h	(revision 151175)
--- gcc/langhooks.h	(working copy)
*************** struct lang_hooks
*** 346,357 ****
    void (*print_error_function) (struct diagnostic_context *, const char *,
  				struct diagnostic_info *);
  
-   /* Called from expr_size to calculate the size of the value of an
-      expression in a language-dependent way.  Returns a tree for the size
-      in bytes.  A frontend can call lhd_expr_size to get the default
-      semantics in cases that it doesn't want to handle specially.  */
-   tree (*expr_size) (const_tree);
- 
    /* Convert a character from the host's to the target's character
       set.  The character should be in what C calls the "basic source
       character set" (roughly, the set of characters defined by plain
--- 346,351 ----
Index: gcc/explow.c
===================================================================
*** gcc/explow.c	(revision 151175)
--- gcc/explow.c	(working copy)
*************** expr_size (tree exp)
*** 246,252 ****
      size = TREE_OPERAND (exp, 1);
    else
      {
!       size = lang_hooks.expr_size (exp);
        gcc_assert (size);
        gcc_assert (size == SUBSTITUTE_PLACEHOLDER_IN_EXPR (size, exp));
      }
--- 246,252 ----
      size = TREE_OPERAND (exp, 1);
    else
      {
!       size = tree_expr_size (exp);
        gcc_assert (size);
        gcc_assert (size == SUBSTITUTE_PLACEHOLDER_IN_EXPR (size, exp));
      }
*************** int_expr_size (tree exp)
*** 266,272 ****
      size = TREE_OPERAND (exp, 1);
    else
      {
!       size = lang_hooks.expr_size (exp);
        gcc_assert (size);
      }
  
--- 266,272 ----
      size = TREE_OPERAND (exp, 1);
    else
      {
!       size = tree_expr_size (exp);
        gcc_assert (size);
      }
  
Index: gcc/langhooks-def.h
===================================================================
*** gcc/langhooks-def.h	(revision 151175)
--- gcc/langhooks-def.h	(working copy)
*************** extern void lhd_incomplete_type_error (c
*** 58,64 ****
  extern tree lhd_type_promotes_to (tree);
  extern void lhd_register_builtin_type (tree, const char *);
  extern bool lhd_decl_ok_for_sibcall (const_tree);
- extern tree lhd_expr_size (const_tree);
  extern size_t lhd_tree_size (enum tree_code);
  extern HOST_WIDE_INT lhd_to_target_charset (HOST_WIDE_INT);
  extern tree lhd_expr_to_decl (tree, bool *, bool *);
--- 58,63 ----
*************** extern void lhd_omp_firstprivatize_type_
*** 100,106 ****
  #define LANG_HOOKS_PRINT_ERROR_FUNCTION lhd_print_error_function
  #define LANG_HOOKS_DECL_PRINTABLE_NAME	lhd_decl_printable_name
  #define LANG_HOOKS_DWARF_NAME		lhd_dwarf_name
- #define LANG_HOOKS_EXPR_SIZE		lhd_expr_size
  #define LANG_HOOKS_TREE_SIZE		lhd_tree_size
  #define LANG_HOOKS_TYPES_COMPATIBLE_P	lhd_types_compatible_p
  #define LANG_HOOKS_BUILTIN_FUNCTION	lhd_builtin_function
--- 99,104 ----
*************** extern tree lhd_make_node (enum tree_cod
*** 249,255 ****
    LANG_HOOKS_DWARF_NAME, \
    LANG_HOOKS_TYPES_COMPATIBLE_P, \
    LANG_HOOKS_PRINT_ERROR_FUNCTION, \
-   LANG_HOOKS_EXPR_SIZE, \
    LANG_HOOKS_TO_TARGET_CHARSET, \
    LANG_HOOKS_ATTRIBUTE_TABLE, \
    LANG_HOOKS_COMMON_ATTRIBUTE_TABLE, \
--- 247,252 ----

Re: [PATCH][C++] Kill expr_size langhook

[Richard Guenther]

On Sat, 29 Aug 2009, Richard Guenther wrote:

> 
> This kills the expr_size langhook.  With the recent C++ FE change
> there isn't any language-specific semantics left that warrant it.
> In particular the C++ FE is already careful enough to use memcpy
> whenever an assignment may cover padding that is used by derived
> classes.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?

Approved by Mike on IRC, applied as r151261.

Richard.

Re: [PATCH][C++] Kill expr_size langhook

[Jason Merrill]

On 08/29/2009 09:47 AM, Richard Guenther wrote:
>
> This kills the expr_size langhook.  With the recent C++ FE change
> there isn't any language-specific semantics left that warrant it.

Perhaps not semantics, but having the langhook return NULL for 
non-byte-copyable classes has caught a number of bugs which otherwise 
would have been hard to detect wrong codegen.

Jason

Re: [PATCH][C++] Kill expr_size langhook

[Richard Guenther]

On Thu, 3 Sep 2009, Jason Merrill wrote:

> On 08/29/2009 09:47 AM, Richard Guenther wrote:
> > 
> > This kills the expr_size langhook.  With the recent C++ FE change
> > there isn't any language-specific semantics left that warrant it.
> 
> Perhaps not semantics, but having the langhook return NULL for
> non-byte-copyable classes has caught a number of bugs which otherwise would
> have been hard to detect wrong codegen.

Those are types with TREE_ADDRESSABLE set, no?  So I guess a strathegical
check somewhere in verify_gimple_* might be as good.

Richard.

Re: [PATCH][C++] Kill expr_size langhook

[Jason Merrill]

On 09/03/2009 01:19 PM, Richard Guenther wrote:
> On Thu, 3 Sep 2009, Jason Merrill wrote:
>
>> On 08/29/2009 09:47 AM, Richard Guenther wrote:
>>>
>>> This kills the expr_size langhook.  With the recent C++ FE change
>>> there isn't any language-specific semantics left that warrant it.
>>
>> Perhaps not semantics, but having the langhook return NULL for
>> non-byte-copyable classes has caught a number of bugs which otherwise would
>> have been hard to detect wrong codegen.
>
> Those are types with TREE_ADDRESSABLE set, no?  So I guess a strategical
> check somewhere in verify_gimple_* might be as good.

Types with TREE_ADDRESSABLE can still be bitwise assigned if they have a 
trivial copy assignment operator, the backend just can't create extra 
copies.

Jason