On Tue, Mar 14, 2017 at 10:36 PM, Bill Schmidt <wschm...@linux.vnet.ibm.com> wrote: > >> On Mar 14, 2017, at 11:07 AM, Bill Schmidt <wschm...@linux.vnet.ibm.com> >> wrote: >>> >>> 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. >> >> Reduced test case: >> >> typedef __builtin_va_list __gnuc_va_list; >> typedef __gnuc_va_list va_list; >> >> void >> foo (va_list args) >> { >> va_list ap; >> __builtin_va_copy (ap, args); >> (void)__builtin_va_arg (ap, int); >> __builtin_va_end(ap); >> } >> > > Perhaps something like this? It appears to be doing better on bootstrap, and > avoids > failure on both the original problem and the new test case above: > > Index: gcc/tree-stdarg.c > =================================================================== > --- gcc/tree-stdarg.c (revision 246109) > +++ gcc/tree-stdarg.c (working copy) > @@ -1058,7 +1058,13 @@ expand_ifn_va_arg_1 (function *fun) > gimplify_assign (lhs, expr, &pre); > } > else > - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue); > + { > + enum gimplify_status status; > + status = gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, > + fb_lvalue | fb_mayfail); > + if (status == GS_ERROR) > + gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_rvalue); > + } > > input_location = saved_location; > pop_gimplify_context (NULL);
No, I was confused in thinking gimplify_expr would handle the case properly. For just gimplifying side-effects we should use the middle-end gimplification machinery: Index: tree-stdarg.c =================================================================== --- tree-stdarg.c (revision 246188) +++ tree-stdarg.c (working copy) @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. #include "gimple-iterator.h" #include "gimple-walk.h" #include "gimplify.h" +#include "gimplify-me.h" #include "tree-into-ssa.h" #include "tree-cfg.h" #include "tree-stdarg.h" @@ -1058,12 +1059,12 @@ expand_ifn_va_arg_1 (function *fun) gimplify_assign (lhs, expr, &pre); } else - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue); + force_gimple_operand (expr, &pre, false, NULL_TREE); input_location = saved_location; pop_gimplify_context (NULL); - gimple_seq_add_seq (&pre, post); + gimple_seq_add_seq_without_update (&pre, post); update_modified_stmts (pre); /* Add the sequence after IFN_VA_ARG. This splits the bb right @@ -1072,11 +1073,10 @@ expand_ifn_va_arg_1 (function *fun) gimple_find_sub_bbs (pre, &i); /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the - bb. */ + bb if we added any stmts. */ unlink_stmt_vdef (stmt); release_ssa_name_fn (fun, gimple_vdef (stmt)); gsi_remove (&i, true); - gcc_assert (gsi_end_p (i)); /* We're walking here into the bbs which contain the expansion of IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs > Bill