public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "vries at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/65887] remove va_arg ap copies
Date: Mon, 27 Apr 2015 07:40:00 -0000	[thread overview]
Message-ID: <bug-65887-4-TrnMqoB88G@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-65887-4@http.gcc.gnu.org/bugzilla/>

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 (&param, 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 (&param, 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.


  parent reply	other threads:[~2015-04-27  7:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-25 14:46 [Bug tree-optimization/65887] New: " 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-65887-4-TrnMqoB88G@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).