On Tue, Mar 14, 2017 at 3:20 PM, Bill Schmidt <wschm...@linux.vnet.ibm.com> wrote: > >> On Mar 14, 2017, at 7:50 AM, Bill Schmidt <wschm...@linux.vnet.ibm.com> >> wrote: >> >> >>> On Mar 14, 2017, at 3:57 AM, Richard Biener <richard.guent...@gmail.com> >>> wrote: >>> >>> On Tue, Mar 14, 2017 at 1:04 AM, Bill Schmidt >>> <wschm...@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? > 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. > + 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 >> >
