* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
@ 2015-04-27 6:59 ` vries at gcc dot gnu.org
2015-04-27 7:40 ` vries at gcc dot gnu.org
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: vries at gcc dot gnu.org @ 2015-04-27 6:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
--- Comment #1 from vries at gcc dot gnu.org ---
Created attachment 35402
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35402&action=edit
patch to remove copyback
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
2015-04-27 6:59 ` [Bug tree-optimization/65887] " vries at gcc dot gnu.org
@ 2015-04-27 7:40 ` vries at gcc dot gnu.org
2015-04-27 9:04 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: vries at gcc dot gnu.org @ 2015-04-27 7:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
--- Comment #2 from vries at gcc dot gnu.org ---
I.
After removing the copyback using attached patch, and marking the va_arg first
argument as addressable as suggested here (
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01314.html ) using this patch (nr
1):
...
@@ -9408,6 +9458,23 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
}
/* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
+ mark_addressable (valist);
ap = build_fold_addr_expr_loc (loc, valist);
tag = build_int_cst (build_pointer_type (type), 0);
*expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
...
we get the desired:
...
e = VA_ARG (&argp, 0B);
e = VA_ARG (&argp, 0B);
...
II.
However, we subsequently run into a verify_gimple_call failure in
gcc.c-torture/execute/va-arg-10.c, for the second argument of this va_copy:
...
__builtin_va_copy (&apc, ap);
...
D.2056 = VA_ARG (&ap, 0B);
...
Presumably because ap is not marked as addressable when gimplifying the
va_copy, but ap is later marked as addressable when gimplifying VA_ARG_EXPR.
With this patch (nr 2), we mark the second va_copy argument as addressable when
gimplifying va_copy:
...
@@ -2339,6 +2340,55 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p,
bool want_value)
&& DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
switch (DECL_FUNCTION_CODE (fndecl))
{
+ case BUILT_IN_VA_COPY:
+ mark_addressable (CALL_EXPR_ARG (*expr_p, 1));
+ break;
case BUILT_IN_VA_START:
{
builtin_va_start_p = TRUE;
...
That indeed prevents the verify_gimple_call error. But the code now contains a
copy:
...
ap.0 = ap;
__builtin_va_copy (&apc, ap.0);
...
D.2057 = VA_ARG (&ap, 0B);
...
The copy in itself does not look incorrect, but we'd rather not have it.
III.
Furthermore, patch nr 1 triggers a verify_gimple_call error on
gcc.c-torture/execute/va-arg-14.c for the first argument of a va_copy:
...
__builtin_va_copy (param, &local);
...
D.1845 = VA_ARG (¶m, 0B);
...
Using this patch (nr 3), we also mark the first argument of the copy as
addressable:
...
@@ -2341,6 +2341,7 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool
want_value)
{
case BUILT_IN_VA_COPY:
mark_addressable (CALL_EXPR_ARG (*expr_p, 1));
+ mark_addressable (CALL_EXPR_ARG (*expr_p, 0));
break;
case BUILT_IN_VA_START:
{
...
That indeed prevents the verify_gimple_call failure. But it results in this
code:
...
param.0 = param;
__builtin_va_copy (param.0, &local);
...
D.1846 = VA_ARG (¶m, 0B);
...
which doesn't look correct: param is unmodified by the va_copy.
OTOH, the obvious tests (execute.exp=va-arg*.c, execute.exp=stdarg*.c,
callabi.exp) are passing, probably because va_list is a pointer type, and
va_copy modifies what param points to.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
2015-04-27 6:59 ` [Bug tree-optimization/65887] " vries at gcc dot gnu.org
2015-04-27 7:40 ` vries at gcc dot gnu.org
@ 2015-04-27 9:04 ` rguenth at gcc dot gnu.org
2015-04-27 15:45 ` vries at gcc dot gnu.org
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: rguenth at gcc dot gnu.org @ 2015-04-27 9:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to vries from comment #2)
> I.
> After removing the copyback using attached patch, and marking the va_arg
> first argument as addressable as suggested here (
> https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01314.html ) using this patch
> (nr 1):
> ...
> @@ -9408,6 +9458,23 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
> }
>
> /* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
> + mark_addressable (valist);
> ap = build_fold_addr_expr_loc (loc, valist);
> tag = build_int_cst (build_pointer_type (type), 0);
> *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap,
> tag);
> ...
>
> we get the desired:
> ...
> e = VA_ARG (&argp, 0B);
> e = VA_ARG (&argp, 0B);
> ...
>
>
> II.
> However, we subsequently run into a verify_gimple_call failure in
> gcc.c-torture/execute/va-arg-10.c, for the second argument of this va_copy:
> ...
> __builtin_va_copy (&apc, ap);
> ...
> D.2056 = VA_ARG (&ap, 0B);
> ...
>
> Presumably because ap is not marked as addressable when gimplifying the
> va_copy, but ap is later marked as addressable when gimplifying VA_ARG_EXPR.
>
> With this patch (nr 2), we mark the second va_copy argument as addressable
> when gimplifying va_copy:
> ...
> @@ -2339,6 +2340,55 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p,
> bool want_value)
> && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> switch (DECL_FUNCTION_CODE (fndecl))
> {
> + case BUILT_IN_VA_COPY:
> + mark_addressable (CALL_EXPR_ARG (*expr_p, 1));
> + break;
> case BUILT_IN_VA_START:
> {
> builtin_va_start_p = TRUE;
> ...
>
> That indeed prevents the verify_gimple_call error. But the code now contains
> a copy:
> ...
> ap.0 = ap;
> __builtin_va_copy (&apc, ap.0);
> ...
> D.2057 = VA_ARG (&ap, 0B);
> ...
> The copy in itself does not look incorrect, but we'd rather not have it.
>
>
> III.
> Furthermore, patch nr 1 triggers a verify_gimple_call error on
> gcc.c-torture/execute/va-arg-14.c for the first argument of a va_copy:
> ...
> __builtin_va_copy (param, &local);
> ...
> D.1845 = VA_ARG (¶m, 0B);
> ...
>
> Using this patch (nr 3), we also mark the first argument of the copy as
> addressable:
> ...
> @@ -2341,6 +2341,7 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p,
> bool want_value)
> {
> case BUILT_IN_VA_COPY:
> mark_addressable (CALL_EXPR_ARG (*expr_p, 1));
> + mark_addressable (CALL_EXPR_ARG (*expr_p, 0));
> break;
> case BUILT_IN_VA_START:
> {
> ...
>
> That indeed prevents the verify_gimple_call failure. But it results in this
> code:
> ...
> param.0 = param;
> __builtin_va_copy (param.0, &local);
> ...
> D.1846 = VA_ARG (¶m, 0B);
> ...
> which doesn't look correct: param is unmodified by the va_copy.
Well, you only get the "copy" if param is of register type (thus a pointer).
So the code is correct I belive.
Rather than marking the va_list arg addressable in all the cases above
you should probably simply ensure the frontend marks it so from the
point it creates a variable with va_list type. This is because even
va_list a1, a2;
a1 = a2;
__builtin_va_arg (a1, ...);
might go wrong when gimplifying a1 = a2.
> OTOH, the obvious tests (execute.exp=va-arg*.c, execute.exp=stdarg*.c,
> callabi.exp) are passing, probably because va_list is a pointer type, and
> va_copy modifies what param points to.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
` (2 preceding siblings ...)
2015-04-27 9:04 ` rguenth at gcc dot gnu.org
@ 2015-04-27 15:45 ` vries at gcc dot gnu.org
2015-04-28 20:59 ` vries at gcc dot gnu.org
2015-04-28 21:03 ` vries at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: vries at gcc dot gnu.org @ 2015-04-27 15:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
--- Comment #4 from vries at gcc dot gnu.org ---
(In reply to Richard Biener from comment #3)
> (In reply to vries from comment #2)
> Rather than marking the va_list arg addressable in all the cases above
> you should probably simply ensure the frontend marks it so from the
> point it creates a variable with va_list type. This is because even
>
> va_list a1, a2;
> a1 = a2;
> __builtin_va_arg (a1, ...);
>
> might go wrong when gimplifying a1 = a2.
>
This seems to do the trick, I'll put it through some more testing:
...
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 9797e17..d6a93d9 10044
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5910,6 +5910,7 @@ set_compound_literal_name (tree decl)
tree
build_va_arg (location_t loc, tree expr, tree type)
{
+ mark_addressable (expr);
expr = build1 (VA_ARG_EXPR, type, expr);
SET_EXPR_LOCATION (expr, loc);
return expr;
...
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
` (3 preceding siblings ...)
2015-04-27 15:45 ` vries at gcc dot gnu.org
@ 2015-04-28 20:59 ` vries at gcc dot gnu.org
2015-04-28 21:03 ` vries at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: vries at gcc dot gnu.org @ 2015-04-28 20:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
--- Comment #5 from vries at gcc dot gnu.org ---
Author: vries
Date: Tue Apr 28 20:58:51 2015
New Revision: 222546
URL: https://gcc.gnu.org/viewcvs?rev=222546&root=gcc&view=rev
Log:
Remove ifn_va_arg ap fixup
2015-04-28 Tom de Vries <tom@codesourcery.com>
PR tree-optimization/65887
* gimplify.c (gimplify_modify_expr): Remove ifn_va_arg ap fixup.
* c-common.c (build_va_arg): Mark va_arg ap argument as addressable.
Modified:
trunk/gcc/ChangeLog
trunk/gcc/c-family/ChangeLog
trunk/gcc/c-family/c-common.c
trunk/gcc/gimplify.c
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug tree-optimization/65887] remove va_arg ap copies
2015-04-25 14:46 [Bug tree-optimization/65887] New: remove va_arg ap copies vries at gcc dot gnu.org
` (4 preceding siblings ...)
2015-04-28 20:59 ` vries at gcc dot gnu.org
@ 2015-04-28 21:03 ` vries at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: vries at gcc dot gnu.org @ 2015-04-28 21:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65887
vries at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #6 from vries at gcc dot gnu.org ---
Patch committed, marking resolved - fixed.
^ permalink raw reply [flat|nested] 7+ messages in thread