On 12-05-15 12:04, Richard Biener wrote:
The fact that we have to handle this specially in both build_va_arg and
>gimplify_va_arg makes me wonder whether we should be dealing with all this in
>build_va_arg already.
>
>That is, determine whether we take the address, and add the address operator
>if so in build_va_arg already. Likewise, determine do_deref and pass that as
>extra operand.
That would certainly be a nice cleanup.
I've implemented this cleanup.
Furthermore, I realized that in expand_ifn_va_arg_1, 'do_deref == 0' disables
code:
...
if (do_deref == integer_one_node)
ap = build_fold_indirect_ref (ap);
...
that has a copy in the target hook:
...
gpr = build3 (COMPONENT_REF, TREE_TYPE (f_gpr),
build_va_arg_indirect_ref (valist), f_gpr, NULL_TREE);
valist = build_va_arg_indirect_ref (valist);
...
It's easier to remove that code from the few target hooks that do that, execute
the 'do_deref == 1' code unconditionally in expand_ifn_va_arg_1 and simply
remove all do_deref propagation (that also means we don't have to add an operand
to VA_ARG_EXPR).
Bootstrapped and reg-tested on x86_64, with and without -m32.
Did partial build and compiled a few va_arg test-cases on the other targets.
OK for trunk?
Thanks,
- Tom
Move array-type va_list handling to build_va_arg
2015-05-13 Tom de Vries <t...@codesourcery.com>
* gimplify.c (gimplify_modify_expr): Remove do_deref handling.
(gimplify_va_arg_expr): Remove do_deref handling. Remove adding of
address operator to va_list operand.
* tree-stdarg.c (expand_ifn_va_arg_1): Do deref of va_list operand
unconditionally.
* config/i386/i386.c (ix86_gimplify_va_arg): Remove deref on va_list
operand.
* config/rs6000/rs6000.c (rs6000_gimplify_va_arg): Same.
* config/s390/s390.c (s390_gimplify_va_arg): Same.
* config/spu/spu.c (spu_gimplify_va_arg_expr): Same.
* c-common.c (build_va_arg_1): New function.
(build_va_arg): Add address operator to va_list operand if necessary.
---
gcc/c-family/c-common.c | 127 +++++++++++++++++++++++++++++++++++++++------
gcc/config/i386/i386.c | 6 +--
gcc/config/rs6000/rs6000.c | 1 -
gcc/config/s390/s390.c | 1 -
gcc/config/spu/spu.c | 1 -
gcc/gimplify.c | 37 ++-----------
gcc/tree-stdarg.c | 14 ++---
7 files changed, 125 insertions(+), 62 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7e5ac72..8f9657d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5915,6 +5915,20 @@ set_compound_literal_name (tree decl)
DECL_NAME (decl) = get_identifier (name);
}
+/* build_va_arg helper function. Return a VA_ARG_EXPR with location LOC, type
+ TYPE and operand OP. */
+
+static tree
+build_va_arg_1 (location_t loc, tree type, tree op)
+{
+ tree expr = build1 (VA_ARG_EXPR, type, op);
+ SET_EXPR_LOCATION (expr, loc);
+ return expr;
+}
+
+/* Return a VA_ARG_EXPR corresponding to a source-level expression
+ va_arg (EXPR, TYPE) at source location LOC. */
+
tree
build_va_arg (location_t loc, tree expr, tree type)
{
@@ -5923,24 +5937,107 @@ build_va_arg (location_t loc, tree expr, tree type)
? NULL_TREE
: targetm.canonical_va_list_type (va_type));
- if (canon_va_type != NULL)
+ if (va_type == error_mark_node
+ || canon_va_type == NULL_TREE)
{
- /* When the va_arg ap argument is a parm decl with declared type va_list,
- and the va_list type is an array, then grokdeclarator changes the type
- of the parm decl to the corresponding pointer type. We know that that
- pointer is constant, so there's no need to modify it, so there's no
- need to pass it around using an address operator, so there's no need to
- mark it addressable. */
- if (!(TREE_CODE (canon_va_type) == ARRAY_TYPE
- && TREE_CODE (va_type) != ARRAY_TYPE))
- /* In gimplify_va_arg_expr we take the address of the ap argument, mark
- it addressable now. */
- mark_addressable (expr);
+ /* Let's handle things neutrallly, if expr:
+ - has undeclared type, or
+ - is not an va_list type. */
+ return build_va_arg_1 (loc, type, expr);
}
- expr = build1 (VA_ARG_EXPR, type, expr);
- SET_EXPR_LOCATION (expr, loc);
- return expr;
+ if (TREE_CODE (canon_va_type) != ARRAY_TYPE)
+ {
+ /* Case 1: Not an array type. */
+
+ /* Take the address, to get '&ap'. */
+ mark_addressable (expr);
+ expr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (expr)), expr);
+
+ /* Verify that &ap is still recognized as having va_list type. */
+ tree canon_expr_type
+ = targetm.canonical_va_list_type (TREE_TYPE (expr));
+ gcc_assert (canon_expr_type != NULL_TREE);
+
+ return build_va_arg_1 (loc, type, expr);
+ }
+
+ /* Case 2: Array type.
+
+ Background:
+
+ For contrast, let's start with the simple case (case 1). If
+ canon_va_type is not an array type, but say a char *, then when
+ passing-by-value a va_list, the type of the va_list param decl is
+ the same as for another va_list decl (all ap's are char *):
+
+ f2_1 (char * ap)
+ D.1815 = VA_ARG (&ap, 0B, 1);
+ return D.1815;
+
+ f2 (int i)
+ char * ap.0;
+ char * ap;
+ __builtin_va_start (&ap, 0);
+ ap.0 = ap;
+ res = f2_1 (ap.0);
+ __builtin_va_end (&ap);
+ D.1812 = res;
+ return D.1812;
+
+ However, if canon_va_type is ARRAY_TYPE, then when passing-by-value a
+ va_list the type of the va_list param decl (case 2b, struct * ap) is not
+ the same as for another va_list decl (case 2a, struct ap[1]).
+
+ f2_1 (struct * ap)
+ D.1844 = VA_ARG (ap, 0B, 0);
+ return D.1844;
+
+ f2 (int i)
+ struct ap[1];
+ __builtin_va_start (&ap, 0);
+ res = f2_1 (&ap);
+ __builtin_va_end (&ap);
+ D.1841 = res;
+ return D.1841;
+
+ Case 2b is different because:
+ - on the callee side, the parm decl has declared type va_list, but
+ grokdeclarator changes the type of the parm decl to a pointer to the
+ array elem type.
+ - on the caller side, the pass-by-value uses &ap.
+
+ We unify these two cases (case 2a: va_list is array type,
+ case 2b: va_list is pointer to array elem type), by adding '&' for the
+ array type case, such that we have a pointer to array elem in both
+ cases. */
+
+ if (TREE_CODE (va_type) == ARRAY_TYPE)
+ {
+ /* Case 2a: va_list is array type. */
+
+ /* Take the address, to get '&ap'. Make sure it's a pointer to array
+ elem type. */
+ mark_addressable (expr);
+ expr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (canon_va_type)),
+ expr);
+
+ /* Verify that &ap is still recognized as having va_list type. */
+ tree canon_expr_type
+ = targetm.canonical_va_list_type (TREE_TYPE (expr));
+ gcc_assert (canon_expr_type != NULL_TREE);
+ }
+ else
+ {
+ /* Case 2b: va_list is pointer to array elem type. */
+ gcc_assert (POINTER_TYPE_P (va_type));
+ gcc_assert (TREE_TYPE (va_type) == TREE_TYPE (canon_va_type));
+
+ /* Don't take the address. We've already got '&ap'. */
+ ;
+ }
+
+ return build_va_arg_1 (loc, type, expr);
}
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index de333d8..4ec469b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -9094,8 +9094,8 @@ ix86_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p,
f_sav = DECL_CHAIN (f_ovf);
gpr = build3 (COMPONENT_REF, TREE_TYPE (f_gpr),
- build_va_arg_indirect_ref (valist), f_gpr, NULL_TREE);
- valist = build_va_arg_indirect_ref (valist);
+ valist, f_gpr, NULL_TREE);
+
fpr = build3 (COMPONENT_REF, TREE_TYPE (f_fpr), valist, f_fpr, NULL_TREE);
ovf = build3 (COMPONENT_REF, TREE_TYPE (f_ovf), valist, f_ovf, NULL_TREE);
sav = build3 (COMPONENT_REF, TREE_TYPE (f_sav), valist, f_sav, NULL_TREE);
@@ -9315,7 +9315,7 @@ ix86_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p,
{
t = build2 (PLUS_EXPR, TREE_TYPE (fpr), fpr,
build_int_cst (TREE_TYPE (fpr), needed_sseregs * 16));
- gimplify_assign (fpr, t, pre_p);
+ gimplify_assign (unshare_expr (fpr), t, pre_p);
}
gimple_seq_add_stmt (pre_p, gimple_build_goto (lab_over));
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ec7332c..077e55a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -11470,7 +11470,6 @@ rs6000_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p,
f_ovf = DECL_CHAIN (f_res);
f_sav = DECL_CHAIN (f_ovf);
- valist = build_va_arg_indirect_ref (valist);
gpr = build3 (COMPONENT_REF, TREE_TYPE (f_gpr), valist, f_gpr, NULL_TREE);
fpr = build3 (COMPONENT_REF, TREE_TYPE (f_fpr), unshare_expr (valist),
f_fpr, NULL_TREE);
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index cbe8e1f..e9a4e70 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9747,7 +9747,6 @@ s390_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p,
f_ovf = DECL_CHAIN (f_fpr);
f_sav = DECL_CHAIN (f_ovf);
- valist = build_va_arg_indirect_ref (valist);
gpr = build3 (COMPONENT_REF, TREE_TYPE (f_gpr), valist, f_gpr, NULL_TREE);
fpr = build3 (COMPONENT_REF, TREE_TYPE (f_fpr), valist, f_fpr, NULL_TREE);
sav = build3 (COMPONENT_REF, TREE_TYPE (f_sav), valist, f_sav, NULL_TREE);
diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c
index a03b87e..7a29df8 100644
--- a/gcc/config/spu/spu.c
+++ b/gcc/config/spu/spu.c
@@ -4065,7 +4065,6 @@ spu_gimplify_va_arg_expr (tree valist, tree type, gimple_seq * pre_p,
f_args = TYPE_FIELDS (TREE_TYPE (va_list_type_node));
f_skip = DECL_CHAIN (f_args);
- valist = build_simple_mem_ref (valist);
args =
build3 (COMPONENT_REF, TREE_TYPE (f_args), valist, f_args, NULL_TREE);
skip =
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 4846478..7d53aa2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4655,13 +4655,13 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
if (TREE_CODE (call) == CALL_EXPR
&& CALL_EXPR_IFN (call) == IFN_VA_ARG)
{
+ int nargs = call_expr_nargs (call);
tree type = TREE_TYPE (call);
tree ap = CALL_EXPR_ARG (call, 0);
tree tag = CALL_EXPR_ARG (call, 1);
- tree do_deref = CALL_EXPR_ARG (call, 2);
tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
- IFN_VA_ARG, type, 4, ap,
- tag, do_deref,
+ IFN_VA_ARG, type,
+ nargs + 1, ap, tag,
vlasize);
tree *call_p = &(TREE_OPERAND (*from_p, 0));
*call_p = newcall;
@@ -9312,7 +9312,7 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
tree promoted_type, have_va_type;
tree valist = TREE_OPERAND (*expr_p, 0);
tree type = TREE_TYPE (*expr_p);
- tree t, tag, ap, do_deref;
+ tree t, tag;
location_t loc = EXPR_LOCATION (*expr_p);
/* Verify that valist is of the proper type. */
@@ -9365,35 +9365,8 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
return GS_ALL_DONE;
}
- /* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
- if (TREE_CODE (have_va_type) == ARRAY_TYPE)
- {
- if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
- {
- /* Take the address, but don't strip it. Gimplify_va_arg_internal
- expects a pointer to array element type. */
- ap = build_fold_addr_expr_loc (loc, valist);
- do_deref = integer_zero_node;
- }
- else
- {
- /* Don't take the address. Gimplify_va_arg_internal expects a pointer
- to array element type, and we already have that.
- See also comment in build_va_arg. */
- ap = valist;
- do_deref = integer_zero_node;
- }
- }
- else
- {
- /* No special handling. Take the address here, note that it needs to be
- stripped before calling gimplify_va_arg_internal. */
- ap = build_fold_addr_expr_loc (loc, valist);
- do_deref = integer_one_node;
- }
tag = build_int_cst (build_pointer_type (type), 0);
- *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 3, ap, tag,
- do_deref);
+ *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, valist, tag);
/* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
needs to be expanded. */
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index f8ff70a..794b94a 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -1042,7 +1042,7 @@ expand_ifn_va_arg_1 (function *fun)
for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
{
gimple stmt = gsi_stmt (i);
- tree ap, expr, lhs, type, do_deref;
+ tree ap, expr, lhs, type;
gimple_seq pre = NULL, post = NULL;
if (!gimple_call_ifn_va_arg_p (stmt))
@@ -1052,19 +1052,15 @@ expand_ifn_va_arg_1 (function *fun)
type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
ap = gimple_call_arg (stmt, 0);
- do_deref = gimple_call_arg (stmt, 2);
- if (do_deref == integer_one_node)
- ap = build_fold_indirect_ref (ap);
+ /* Balanced out the &ap, usually added by build_va_arg. */
+ ap = build_fold_indirect_ref (ap);
push_gimplify_context (false);
/* Make it easier for the backends by protecting the valist argument
from multiple evaluations. */
- if (do_deref == integer_one_node)
- gimplify_expr (&ap, &pre, &post, is_gimple_min_lval, fb_lvalue);
- else
- gimplify_expr (&ap, &pre, &post, is_gimple_val, fb_rvalue);
+ gimplify_expr (&ap, &pre, &post, is_gimple_min_lval, fb_lvalue);
expr = targetm.gimplify_va_arg_expr (ap, type, &pre, &post);
@@ -1074,7 +1070,7 @@ expand_ifn_va_arg_1 (function *fun)
unsigned int nargs = gimple_call_num_args (stmt);
gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type));
- if (nargs == 4)
+ if (nargs == 3)
{
/* We've transported the size of with WITH_SIZE_EXPR here as
the last argument of the internal fn call. Now reinstate
--
1.9.1
