diff mbox series

Fix gimplify_one_sizepos (PR libgomp/83590, take 4)

Message ID 20180116145426.GK2063@tucnak
State New
Headers show
Series Fix gimplify_one_sizepos (PR libgomp/83590, take 4) | expand

Commit Message

Jakub Jelinek Jan. 16, 2018, 2:54 p.m. UTC
Hi!

After lengthy IRC discussions, here is an updated patch, which should also
fix the problem that variably_modified_type_p on a REAL_TYPE returns true
even when it has constant maximum and minimum.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-01-16  Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenth@suse.de>

	PR libgomp/83590
	* gimplify.c (gimplify_one_sizepos): For is_gimple_constant (expr)
	return early, inline manually is_gimple_sizepos.  Make sure if we
	call gimplify_expr we don't end up with a gimple constant.
	* tree.c (variably_modified_type_p): Don't return true for
	is_gimple_constant (_t).  Inline manually is_gimple_sizepos.
	* gimplify.h (is_gimple_sizepos): Remove.


	Jakub

Comments

Richard Biener Jan. 16, 2018, 3:07 p.m. UTC | #1
On Tue, 16 Jan 2018, Jakub Jelinek wrote:

> Hi!
> 
> After lengthy IRC discussions, here is an updated patch, which should also
> fix the problem that variably_modified_type_p on a REAL_TYPE returns true
> even when it has constant maximum and minimum.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2018-01-16  Jakub Jelinek  <jakub@redhat.com>
> 	    Richard Biener  <rguenth@suse.de>
> 
> 	PR libgomp/83590
> 	* gimplify.c (gimplify_one_sizepos): For is_gimple_constant (expr)
> 	return early, inline manually is_gimple_sizepos.  Make sure if we
> 	call gimplify_expr we don't end up with a gimple constant.
> 	* tree.c (variably_modified_type_p): Don't return true for
> 	is_gimple_constant (_t).  Inline manually is_gimple_sizepos.
> 	* gimplify.h (is_gimple_sizepos): Remove.
> 
> --- gcc/gimplify.c.jj	2018-01-12 16:38:50.705238254 +0100
> +++ gcc/gimplify.c	2018-01-16 12:21:15.895859416 +0100
> @@ -12562,7 +12562,10 @@ gimplify_one_sizepos (tree *expr_p, gimp
>       a VAR_DECL.  If it's a VAR_DECL from another function, the gimplifier
>       will want to replace it with a new variable, but that will cause problems
>       if this type is from outside the function.  It's OK to have that here.  */
> -  if (is_gimple_sizepos (expr))
> +  if (expr == NULL_TREE
> +      || is_gimple_constant (expr)
> +      || TREE_CODE (expr) == VAR_DECL
> +      || CONTAINS_PLACEHOLDER_P (expr))
>      return;
>  
>    *expr_p = unshare_expr (expr);
> @@ -12570,6 +12573,12 @@ gimplify_one_sizepos (tree *expr_p, gimp
>    /* SSA names in decl/type fields are a bad idea - they'll get reclaimed
>       if the def vanishes.  */
>    gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue, false);
> +
> +  /* If expr wasn't already is_gimple_sizepos or is_gimple_constant from the
> +     FE, ensure that it is a VAR_DECL, otherwise we might handle some decls
> +     as gimplify_vla_decl even when they would have all sizes INTEGER_CSTs.  */
> +  if (is_gimple_constant (*expr_p))
> +    *expr_p = get_initialized_tmp_var (*expr_p, stmt_p, NULL, false);
>  }
>  
>  /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node
> --- gcc/tree.c.jj	2018-01-15 10:01:40.830186474 +0100
> +++ gcc/tree.c	2018-01-16 12:24:11.254821615 +0100
> @@ -8825,11 +8825,12 @@ variably_modified_type_p (tree type, tre
>    do { tree _t = (T);							\
>      if (_t != NULL_TREE							\
>  	&& _t != error_mark_node					\
> -	&& TREE_CODE (_t) != INTEGER_CST				\
> +	&& !CONSTANT_CLASS_P (_t)					\
>  	&& TREE_CODE (_t) != PLACEHOLDER_EXPR				\
>  	&& (!fn								\
>  	    || (!TYPE_SIZES_GIMPLIFIED (type)				\
> -		&& !is_gimple_sizepos (_t))				\
> +		&& (TREE_CODE (_t) != VAR_DECL				\
> +		    && !CONTAINS_PLACEHOLDER_P (_t)))			\
>  	    || walk_tree (&_t, find_var_from_fn, fn, NULL)))		\
>        return true;  } while (0)
>  
> --- gcc/gimplify.h.jj	2018-01-03 10:19:53.757533721 +0100
> +++ gcc/gimplify.h	2018-01-16 12:24:51.995812831 +0100
> @@ -85,23 +85,4 @@ extern enum gimplify_status gimplify_va_
>  						  gimple_seq *);
>  gimple *gimplify_assign (tree, tree, gimple_seq *);
>  
> -/* Return true if gimplify_one_sizepos doesn't need to gimplify
> -   expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
> -   fields).  */
> -
> -static inline bool
> -is_gimple_sizepos (tree expr)
> -{
> -  /* gimplify_one_sizepos doesn't need to do anything if the value isn't there,
> -     is constant, or contains A PLACEHOLDER_EXPR.  We also don't want to do
> -     anything if it's already a VAR_DECL.  If it's a VAR_DECL from another
> -     function, the gimplifier will want to replace it with a new variable,
> -     but that will cause problems if this type is from outside the function.
> -     It's OK to have that here.  */
> -  return (expr == NULL_TREE
> -	  || TREE_CODE (expr) == INTEGER_CST
> -	  || TREE_CODE (expr) == VAR_DECL
> -	  || CONTAINS_PLACEHOLDER_P (expr));
> -}                                        
> -
>  #endif /* GCC_GIMPLIFY_H */
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/gimplify.c.jj	2018-01-12 16:38:50.705238254 +0100
+++ gcc/gimplify.c	2018-01-16 12:21:15.895859416 +0100
@@ -12562,7 +12562,10 @@  gimplify_one_sizepos (tree *expr_p, gimp
      a VAR_DECL.  If it's a VAR_DECL from another function, the gimplifier
      will want to replace it with a new variable, but that will cause problems
      if this type is from outside the function.  It's OK to have that here.  */
-  if (is_gimple_sizepos (expr))
+  if (expr == NULL_TREE
+      || is_gimple_constant (expr)
+      || TREE_CODE (expr) == VAR_DECL
+      || CONTAINS_PLACEHOLDER_P (expr))
     return;
 
   *expr_p = unshare_expr (expr);
@@ -12570,6 +12573,12 @@  gimplify_one_sizepos (tree *expr_p, gimp
   /* SSA names in decl/type fields are a bad idea - they'll get reclaimed
      if the def vanishes.  */
   gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue, false);
+
+  /* If expr wasn't already is_gimple_sizepos or is_gimple_constant from the
+     FE, ensure that it is a VAR_DECL, otherwise we might handle some decls
+     as gimplify_vla_decl even when they would have all sizes INTEGER_CSTs.  */
+  if (is_gimple_constant (*expr_p))
+    *expr_p = get_initialized_tmp_var (*expr_p, stmt_p, NULL, false);
 }
 
 /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node
--- gcc/tree.c.jj	2018-01-15 10:01:40.830186474 +0100
+++ gcc/tree.c	2018-01-16 12:24:11.254821615 +0100
@@ -8825,11 +8825,12 @@  variably_modified_type_p (tree type, tre
   do { tree _t = (T);							\
     if (_t != NULL_TREE							\
 	&& _t != error_mark_node					\
-	&& TREE_CODE (_t) != INTEGER_CST				\
+	&& !CONSTANT_CLASS_P (_t)					\
 	&& TREE_CODE (_t) != PLACEHOLDER_EXPR				\
 	&& (!fn								\
 	    || (!TYPE_SIZES_GIMPLIFIED (type)				\
-		&& !is_gimple_sizepos (_t))				\
+		&& (TREE_CODE (_t) != VAR_DECL				\
+		    && !CONTAINS_PLACEHOLDER_P (_t)))			\
 	    || walk_tree (&_t, find_var_from_fn, fn, NULL)))		\
       return true;  } while (0)
 
--- gcc/gimplify.h.jj	2018-01-03 10:19:53.757533721 +0100
+++ gcc/gimplify.h	2018-01-16 12:24:51.995812831 +0100
@@ -85,23 +85,4 @@  extern enum gimplify_status gimplify_va_
 						  gimple_seq *);
 gimple *gimplify_assign (tree, tree, gimple_seq *);
 
-/* Return true if gimplify_one_sizepos doesn't need to gimplify
-   expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
-   fields).  */
-
-static inline bool
-is_gimple_sizepos (tree expr)
-{
-  /* gimplify_one_sizepos doesn't need to do anything if the value isn't there,
-     is constant, or contains A PLACEHOLDER_EXPR.  We also don't want to do
-     anything if it's already a VAR_DECL.  If it's a VAR_DECL from another
-     function, the gimplifier will want to replace it with a new variable,
-     but that will cause problems if this type is from outside the function.
-     It's OK to have that here.  */
-  return (expr == NULL_TREE
-	  || TREE_CODE (expr) == INTEGER_CST
-	  || TREE_CODE (expr) == VAR_DECL
-	  || CONTAINS_PLACEHOLDER_P (expr));
-}                                        
-
 #endif /* GCC_GIMPLIFY_H */