diff mbox

Fix PR79908

Message ID CAFiYyc0wg05HaNXvuPBy+SXHqEEge9eDpKR9XWLRpLgrqeRQHg@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener March 14, 2017, 8:57 a.m. UTC 
On Tue, Mar 14, 2017 at 1:04 AM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79908 shows a case where
> pass_stdarg ICEs attempting to gimplify a COMPLEX_EXPR with side
> effects as an lvalue.  The expression is not addressable, so the
> gimplification fails.  This patch says, hey, don't do that!
>
> The resulting GIMPLE looks fine afterward.  Bootstrapped and tested
> on powerpc64le-unknown-linux-gnu with no regressions.  Is this ok
> for trunk?
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2017-03-13  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR tree-optimization/79908
>         * tree-stdarg.c (expand_ifn_va_arg_1): Don't force something to be
>         an lvalue that isn't addressable.
>
> [gcc/testsuite]
>
> 2017-03-13  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR tree-optimization/79908
>         * gcc.dg/torture/pr79908.c: New file.
>
>
> Index: gcc/testsuite/gcc.dg/torture/pr79908.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr79908.c      (nonexistent)
> +++ gcc/testsuite/gcc.dg/torture/pr79908.c      (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +
> +/* Used to fail in the stdarg pass before fix for PR79908.  */
> +
> +typedef __builtin_va_list __gnuc_va_list;
> +typedef __gnuc_va_list va_list;
> +
> +void testva (int n, ...)
> +{
> +  va_list ap;
> +  _Complex int i = __builtin_va_arg (ap, _Complex int);
> +}
> Index: gcc/tree-stdarg.c
> ===================================================================
> --- gcc/tree-stdarg.c   (revision 246109)
> +++ gcc/tree-stdarg.c   (working copy)
> @@ -1057,7 +1057,7 @@ expand_ifn_va_arg_1 (function *fun)
>                types.  */
>             gimplify_assign (lhs, expr, &pre);
>           }
> -       else
> +       else if (is_gimple_addressable (expr))
>           gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);

This is wrong - we lose side-effects this way and the only reason we gimplify
is to _not_ lose them.

Better is sth like



>
>         input_location = saved_location;
>

Comments

Bill Schmidt March 14, 2017, 12:50 p.m. UTC | #1 
> On Mar 14, 2017, at 3:57 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, Mar 14, 2017 at 1:04 AM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>> 
>> Index: gcc/tree-stdarg.c
>> ===================================================================
>> --- gcc/tree-stdarg.c   (revision 246109)
>> +++ gcc/tree-stdarg.c   (working copy)
>> @@ -1057,7 +1057,7 @@ expand_ifn_va_arg_1 (function *fun)
>>               types.  */
>>            gimplify_assign (lhs, expr, &pre);
>>          }
>> -       else
>> +       else if (is_gimple_addressable (expr))
>>          gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
> 
> This is wrong - we lose side-effects this way and the only reason we gimplify
> is to _not_ lose them.
> 
> Better is sth like
> 
> Index: gcc/tree-stdarg.c
> ===================================================================
> --- gcc/tree-stdarg.c   (revision 246082)
> +++ gcc/tree-stdarg.c   (working copy)
> @@ -1058,7 +1058,7 @@ expand_ifn_va_arg_1 (function *fun)
>            gimplify_assign (lhs, expr, &pre);
>          }
>        else
> -         gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
> +         gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_either);
> 
>        input_location = saved_location;
>        pop_gimplify_context (NULL);

OK, thanks for the explanation.  Unfortunately this fails bootstrap with a failed 
assert a little later.  I'll dig further.

Bill
diff mbox

Patch

Index: gcc/tree-stdarg.c
===================================================================
--- gcc/tree-stdarg.c   (revision 246082)
+++ gcc/tree-stdarg.c   (working copy)
@@ -1058,7 +1058,7 @@  expand_ifn_va_arg_1 (function *fun)
            gimplify_assign (lhs, expr, &pre);
          }
        else
-         gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
+         gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_either);

        input_location = saved_location;
        pop_gimplify_context (NULL);