> On Mar 17, 2017, at 9:52 AM, Bill Schmidt <wschm...@linux.vnet.ibm.com> wrote:
>
> Hi,
>
>> On Mar 17, 2017, at 6:44 AM, Richard Biener <richard.guent...@gmail.com>
>> wrote:
>>
>> 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
>
> Looks good, but for some reason I hit a segfault in the linker building
> gengtype when I tried to bootstrap with this. I assume it's something
> latent and unrelated, but will need to investigate.
Ah, I misread the build log. The link of gengtype succeeds, but gengtype
has apparently been miscompiled (stage2) as it tries to call strlen() with
an address of zero. So something wrong with this patch. Looks like the
miscompilation is of libiberty_vprintf_buffer_size in
libiberty/vprintf-support.c.
I looked at the before and after dumps and we see that VA_ARGs with
no lhs are just removed from the code, instead of expanding the advance
of the va pointer.
Before:
<L16> [1.39%]:
VA_ARG (&ap, 0B, 0B);
goto <bb 22> (<L31>); [100.00%]
<L23> [1.39%]:
VA_ARG (&ap, 0B, 0B);
total_width_89 = total_width_17 + 337;
goto <bb 22> (<L31>); [100.00%]
After:
<L16> [1.39%]:
goto <bb 22> (<L31>); [100.00%]
<L23> [1.39%]:
total_width_89 = total_width_17 + 337;
goto <bb 22> (<L31>); [100.00%]
Thanks,
Bill
