This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR79908
> On Mar 14, 2017, at 9:25 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Tue, Mar 14, 2017 at 3:20 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>>
>>> On Mar 14, 2017, at 7:50 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>>>
>>>
>>>> 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.
>>
>> Looks like is_gimple_val is too restrictive for MEM_REFs, for which is_gimple_lvalue
>> passes. Trying this now:
>
> Hmm, it should simply gimplify to a load if it's not aggregate. Can
> you share a testcase
> where it doesn't work?
So taking a step back...
The original problem is that most VA_ARGs get gimplified to a single SSA var,
which always passes the is_gimple_addressable check, so if the LHS has either
been cast away or gone dead, we get a nice lvalue to work with and everyone is
happy.
For a VA_ARG of _Complex type, gimplification instead produces a
COMPLEX_EXPR <a, b> where a and b have been reduced to SSA vars.
The is_gimple_addressable check does not pass in this case, so we can't
find the mandated lvalue and ICE in the gimplifier after doing all the work.
I suppose we could paper over this with:
Index: gcc/tree-stdarg.c
===================================================================
--- gcc/tree-stdarg.c (revision 246109)
+++ gcc/tree-stdarg.c (working copy)
@@ -1058,7 +1058,10 @@ 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_lvalue,
+ fb_lvalue | fb_mayfail);
...but that is perhaps opening the gate too wide (though we could throw
an error if gimplify_expr fails and expr is not a COMPLEX_EXPR). The
COMPLEX_EXPR really is not addressable, but forcing any COMPLEX_EXPR
to a variable in gimplify_expr also seems like overkill. (All of the tcc_binary
expressions are treated the same; we would fail if somebody asked for an
lvalue of a PLUS_EXPR just the same.) So offhand I don't see anything
better than the above.
Thoughts?
Thanks,
Bill