On Tue, Feb 10, 2015 at 10:22 AM, Tom de Vries <tom_devr...@mentor.com> wrote:
> [ was: Re: pass_stdarg problem when run after pass_lim ]
>
> On 03-02-15 14:36, Michael Matz wrote:
>>
>> Hi,
>>
>> On Tue, 3 Feb 2015, Tom de Vries wrote:
>>
>>> Ironically, that fix breaks the va_list_gpr/fpr_size optimization, so
>>> I've disabled that by default for now.
>>>
>>> I've done a non-bootstrap and bootstrap build using all languages.
>>>
>>> The non-bootstrap test shows (at least) two classes of real failures:
>>> - gcc.c-torture/execute/20020412-1.c, gcc.target/i386/memcpy-strategy-4.c
>>> and
>>> gcc.dg/lto/20090706-1_0.c.
>>> These are test-cases with vla as va_arg argument. It ICEs in
>>> force_constant_size with call stack
>>> gimplify_va_arg_expr -> create_tmp_var -> gimple_add_tmp_var ->
>>> force_constant_size
>>
>>
>> Hah, yeah, that's the issue I remembered with create_tmp_var. This needs
>> a change in how to represent the va_arg "call", because the LHS can't be a
>> temporary that's copied to the real LHS afterwards.
>>
>>> - most/all va_arg tests with flto, f.i. gcc.c-torture/execute/stdarg-1.c.
>>> It segfaults in lto1 during pass_stdarg, in gimplify_va_arg_internal
>>> when
>>> accessing have_va_type which is NULL_TREE after
>>> 'have_va_type = targetm.canonical_va_list_type (have_va_type)'.
>>>
>>> I don't think the flto issue is difficult to fix. But the vla issue
>>> probably needs more time than I have available right now.
>>
>>
>> I'll think about this.
>>
>
> A status update. I've worked a bit more on this patch series, latest version
> available at vries/expand-va-arg-at-pass-stdarg (and last patch in series
> attached).
>
> I've done a non-bootstrap x86_64 build for all languages and ran the
> regression testsuite for unix/ unix/-m32. I'm left with one failing
> test-case.
> [ Of course there are a bunch of scan-dump-tree failures because the
> va_list_gpr/fpr_size optimization is switched off. ]
>
> The patch series handles things now as follows. At gimplify_va_arg_expr, the
> VA_ARG expr is not gimplified, but replaced by the internal function call.
>
> That is passed upwards to gimplify_modify_expr, which does the actual
> gimplification. In this function we have sufficient scope of the problem to
> deal with it.
>
> I've added two modifications to gimplify_modify_expr:
> - the WITH_SIZE_EXPR in which the CALL_TREE is wrapped, is dropped after
> gimplification, but we need the size expression at expansion in
> pass_stdarg.
> So I added the size expression as argument to the internal function.
> [ And at pass_stdarg::execute, we wrap the result of
> gimplify_va_arg_internal
> in a WITH_SIZE_EXPR before generating the assign to the lhs ]
> - we detect after gimplify_arg (&ap) whether it created a copy ap.1 of ap,
> rather than use ap itself, and if so, we copy the value back from ap.1 to
> ap
> after va_arg.
>
> I've worked around the issue of targetm.canonical_va_list_type
> (have_va_type) returning NULL_TREE in gimplify_va_arg_internal during lto1,
> by simply working with the original type in that case:
> ...
> tree have_va_type = TREE_TYPE (valist);
> tree cano_type = targetm.canonical_va_list_type (have_va_type);
>
> if (cano_type != NULL_TREE)
> have_va_type = cano_type;
> ...
>
> I'm not really sure yet why std_gimplify_va_arg_expr has a part commented
> out. Michael, can you comment?
>
>
> The single failing testcase (both with and without -m32) is
> g++.dg/torture/pr45843.C:
> ...
> ./gcc/testsuite/g++/g++.sum:FAIL: g++.dg/torture/pr45843.C -O2 -flto
> -fno-use-linker-plugin -flto-partition=none (internal compiler error)
> ...
>
> The failure looks like this (it happens during the gimplify_assign after
> calling gimplify_va_arg_internal):
> ...
> src/gcc/testsuite/g++.dg/torture/pr45843.C: In function ‘foo(int, ...)’:
> src/gcc/testsuite/g++.dg/torture/pr45843.C:11:1: internal compiler error:
> Segmentation fault
> 0x10a5b04 crash_signal
> src/gcc/toplev.c:383
> 0x6a8985 tree_check(tree_node*, char const*, int, char const*, tree_code)
> src/gcc/tree.h:2845
> 0x7c2f6a is_really_empty_class(tree_node*)
> src/gcc/cp/class.c:7923
> 0x923855 cp_gimplify_expr(tree_node**, gimple_statement_base**,
> gimple_statement_base**)
> src/gcc/cp/cp-gimplify.c:625
> 0xd34641 gimplify_expr(tree_node**, gimple_statement_base**,
> gimple_statement_base**, bool (*)(tree_node*), int)
> src/gcc/gimplify.c:7843
> 0xd2a04d gimplify_stmt(tree_node**, gimple_statement_base**)
> src/gcc/gimplify.c:5551
> 0xd173e3 gimplify_and_add(tree_node*, gimple_statement_base**)
> src/gcc/gimplify.c:419
> 0xd39c94 gimplify_assign(tree_node*, tree_node*, gimple_statement_base**)
> src/gcc/gimplify.c:9452
> 0x130ad18 execute
> src/gcc/tree-stdarg.c:779
> ...
>
> The testcase contains this struct:
> ...
> struct S { struct T { } a[14]; char b; };
> ...
>
> and uses that struct S as type in va_arg:
> ...
> arg = va_arg (ap, struct S);
> ...
>
> The segfault happens because we're calling is_really_empty_class for struct
> S, and TYPE_BINFO is NULL_TREE, which causes BINFO_BASE_ITERATE to segfault.
> I'm not sure yet what this issue is or how this is supposed to be fixed.
That's probably free_lang_data being more aggressive after Honza
fiddled with BINFOs? That is - the gimplifications called from tree-stdarg.c
(and others from the middle-end) should never call back into the frontend
via langhooks...
Richard.
> Thanks,
> - Tom
>
