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

Reply via email to