RFA (gimplify.h) Fix incorrect cp/ use of get_formal_tmp_var.

Message ID 20191014202503.1542-1-jason@redhat.com
State New
Headers show
Series
  • RFA (gimplify.h) Fix incorrect cp/ use of get_formal_tmp_var.
Related show

Commit Message

Jason Merrill Oct. 14, 2019, 8:25 p.m.
The comment for get_formal_tmp_var says that it shouldn't be used for
expressions whose value might change between initialization and use, and in
this case we're creating a temporary precisely because the value might
change, so we should use get_initialized_tmp_var instead.

I also noticed that many callers of get_initialized_tmp_var pass NULL for
post_p, so it seems appropriate to make it a default argument.  OK for trunk?

Tested x86_64-pc-linux-gnu.

gcc/cp/
	* cp-gimplify.c (cp_gimplify_expr): Use get_initialized_tmp_var.
gcc/
	* gimplify.h (get_initialized_tmp_var): Add default argument to
	post_p.
---
 gcc/gimplify.h       | 2 +-
 gcc/cp/cp-gimplify.c | 2 +-
 gcc/gimplify.c       | 5 +++--
 3 files changed, 5 insertions(+), 4 deletions(-)


base-commit: aa45db50a034b266c338b55dee1b412178ea84a7
-- 
2.18.1

Comments

Aldy Hernandez Oct. 15, 2019, 7:03 a.m. | #1
On 10/14/19 4:25 PM, Jason Merrill wrote:
> The comment for get_formal_tmp_var says that it shouldn't be used for

> expressions whose value might change between initialization and use, and in

> this case we're creating a temporary precisely because the value might

> change, so we should use get_initialized_tmp_var instead.

> 

> I also noticed that many callers of get_initialized_tmp_var pass NULL for

> post_p, so it seems appropriate to make it a default argument.  OK for trunk?


BTW, a patch removing all those (now) unnecessary NULLs from 
get_initialized_tmp_var calls is pre-approved ;-).

What a pity that a lot of callers that pass NULL for post_p, pass FALSE 
for ssa, otherwise those could be cleaned up.

OK.

Thanks.
Aldy
Richard Biener Oct. 15, 2019, 9:25 a.m. | #2
On Mon, Oct 14, 2019 at 10:25 PM Jason Merrill <jason@redhat.com> wrote:
>

> The comment for get_formal_tmp_var says that it shouldn't be used for

> expressions whose value might change between initialization and use, and in

> this case we're creating a temporary precisely because the value might

> change, so we should use get_initialized_tmp_var instead.

>

> I also noticed that many callers of get_initialized_tmp_var pass NULL for

> post_p, so it seems appropriate to make it a default argument.  OK for trunk?


OK.

> Tested x86_64-pc-linux-gnu.

>

> gcc/cp/

>         * cp-gimplify.c (cp_gimplify_expr): Use get_initialized_tmp_var.

> gcc/

>         * gimplify.h (get_initialized_tmp_var): Add default argument to

>         post_p.

> ---

>  gcc/gimplify.h       | 2 +-

>  gcc/cp/cp-gimplify.c | 2 +-

>  gcc/gimplify.c       | 5 +++--

>  3 files changed, 5 insertions(+), 4 deletions(-)

>

> diff --git a/gcc/gimplify.h b/gcc/gimplify.h

> index 1070006374a..6c997a769cd 100644

> --- a/gcc/gimplify.h

> +++ b/gcc/gimplify.h

> @@ -57,7 +57,7 @@ extern gbind *gimple_current_bind_expr (void);

>  extern vec<gbind *> gimple_bind_expr_stack (void);

>  extern void gimplify_and_add (tree, gimple_seq *);

>  extern tree get_formal_tmp_var (tree, gimple_seq *);

> -extern tree get_initialized_tmp_var (tree, gimple_seq *, gimple_seq *,

> +extern tree get_initialized_tmp_var (tree, gimple_seq *, gimple_seq * = NULL,

>                                      bool = true);

>  extern void declare_vars (tree, gimple *, bool);

>  extern void gimple_add_tmp_var (tree);

> diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c

> index 154fa70ec06..80754b930b9 100644

> --- a/gcc/cp/cp-gimplify.c

> +++ b/gcc/cp/cp-gimplify.c

> @@ -767,7 +767,7 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)

>                 && (TREE_CODE (op1) == CALL_EXPR

>                     || (SCALAR_TYPE_P (TREE_TYPE (op1))

>                         && !TREE_CONSTANT (op1))))

> -        TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (op1, pre_p);

> +        TREE_OPERAND (*expr_p, 1) = get_initialized_tmp_var (op1, pre_p);

>        }

>        ret = GS_OK;

>        break;

> diff --git a/gcc/gimplify.c b/gcc/gimplify.c

> index 836706961f3..7f9100ba97d 100644

> --- a/gcc/gimplify.c

> +++ b/gcc/gimplify.c

> @@ -661,8 +661,9 @@ get_formal_tmp_var (tree val, gimple_seq *pre_p)

>     are as in gimplify_expr.  */

>

>  tree

> -get_initialized_tmp_var (tree val, gimple_seq *pre_p, gimple_seq *post_p,

> -                        bool allow_ssa)

> +get_initialized_tmp_var (tree val, gimple_seq *pre_p,

> +                        gimple_seq *post_p /* = NULL */,

> +                        bool allow_ssa /* = true */)

>  {

>    return internal_get_tmp_var (val, pre_p, post_p, false, allow_ssa);

>  }

>

> base-commit: aa45db50a034b266c338b55dee1b412178ea84a7

> --

> 2.18.1

>

Patch

diff --git a/gcc/gimplify.h b/gcc/gimplify.h
index 1070006374a..6c997a769cd 100644
--- a/gcc/gimplify.h
+++ b/gcc/gimplify.h
@@ -57,7 +57,7 @@  extern gbind *gimple_current_bind_expr (void);
 extern vec<gbind *> gimple_bind_expr_stack (void);
 extern void gimplify_and_add (tree, gimple_seq *);
 extern tree get_formal_tmp_var (tree, gimple_seq *);
-extern tree get_initialized_tmp_var (tree, gimple_seq *, gimple_seq *,
+extern tree get_initialized_tmp_var (tree, gimple_seq *, gimple_seq * = NULL,
 				     bool = true);
 extern void declare_vars (tree, gimple *, bool);
 extern void gimple_add_tmp_var (tree);
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 154fa70ec06..80754b930b9 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -767,7 +767,7 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 		&& (TREE_CODE (op1) == CALL_EXPR
 		    || (SCALAR_TYPE_P (TREE_TYPE (op1))
 			&& !TREE_CONSTANT (op1))))
-	 TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (op1, pre_p);
+	 TREE_OPERAND (*expr_p, 1) = get_initialized_tmp_var (op1, pre_p);
       }
       ret = GS_OK;
       break;
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 836706961f3..7f9100ba97d 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -661,8 +661,9 @@  get_formal_tmp_var (tree val, gimple_seq *pre_p)
    are as in gimplify_expr.  */
 
 tree
-get_initialized_tmp_var (tree val, gimple_seq *pre_p, gimple_seq *post_p,
-			 bool allow_ssa)
+get_initialized_tmp_var (tree val, gimple_seq *pre_p,
+			 gimple_seq *post_p /* = NULL */,
+			 bool allow_ssa /* = true */)
 {
   return internal_get_tmp_var (val, pre_p, post_p, false, allow_ssa);
 }