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?
Your suggestion failed bootstrap in libiberty on vprintf-support.c. Compilation failed with:
/home/wschmidt/gcc/build/gcc-mainline-test2-debug/gcc/xgcc -B/home/wschmidt/gcc/build/gcc-mainline-test2-debug/gcc/ -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/bin/ -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/bin/ -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/lib/ -isystem /home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/include -isystem /home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/sys-include -c -DHAVE_CONFIG_H -g -O2 -gtoggle -I. -I/home/wschmidt/gcc/gcc-mainline-test2/libiberty/../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -Wshadow=local -pedantic -D_GNU_SOURCE -fPIC /home/wschmidt/gcc/gcc-mainline-test2/libiberty/vprintf-support.c -o pic/vprintf-support.o
The initial expression being gimplified is ADDR_EXPR (VAR_DECL (ap)). Gimplification
turns this into MEM_REF (VAR_DECL (D.4274), 0), and the is_gimple_val test fails on that.
Bill
>
>> Index: gcc/tree-stdarg.c
>> ===================================================================
>> --- gcc/tree-stdarg.c (revision 246109)
>> +++ gcc/tree-stdarg.c (working copy)
>> @@ -1057,8 +1057,10 @@ expand_ifn_va_arg_1 (function *fun)
>> types. */
>> gimplify_assign (lhs, expr, &pre);
>> }
>> + else if (is_gimple_addressable (expr))
>
> I believe any is_gimple_addressable check is flawed.
OK, just wanted to try something quick and dirty before your end of day. Not sure how
else to deal with this.
Bill
>
>> + gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
>> else
>> - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
>> + gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_rvalue);
>>
>> input_location = saved_location;
>> pop_gimplify_context (NULL);
>>
>>
>>>
>>> Bill