Re: Postpone expanding va_arg until pass_stdarg

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Feb 12, 2015 at 11:51 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 12-02-15 14:57, Michael Matz wrote:
>>
>> Hi,
>>
>> On Wed, 11 Feb 2015, Tom de Vries wrote:
>>
>>>> My idea was to not generate temporaries and hence copies for
>>>> non-scalar types, but rather construct the "result" of va_arg directly
>>>> into the original LHS (that would then also trivially solve the
>>>> problem of nno-copyable types).
>>>
>>>
>>> The copy mentioned here is of ap, not of the result of va_arg.
>>
>>
>> Whoops, I misread, yes.  Thanks.
>>
>
> Hi,
>
> Btw, I'm not happy about the ap copies, but I haven't been able to get rid
> of them.
>
>
>>>>> I'm not really sure yet why std_gimplify_va_arg_expr has a part
>>>>> commented out. Michael, can you comment?
>>>>
>>>>
>>>> I think I did that because of SSA form.  The old sequence calculated
>>>>
>>>>     vatmp = valist;
>>>>     vatmp = vatmp + boundary-1
>>>>     vatmp = vatmp & -boundary
>>>>
>>>> (where the local variable in that function 'valist_tmp' is the tree
>>>> VAR_DECL 'vatmp') and then continue to use valist_tmp.  When in SSA form
>>>> the gimplifier will rewrite this into:
>>>>
>>>>     vatmp_1 = valist;
>>>>     vatmp_2 = vatmp_1 + boundary-1
>>>>     vatmp_3 = vatmp_2 & -boundary
>>>>
>>>> but the local valist_tmp variable will continue to be the VAR_DECL, not
>>>> the vatmp_3 ssa name.  Basically whenever one gimplifies a MODIFY_EXPR
>>>> while in SSA form it's suspicious.  So the new code simply build the
>>>> expression:
>>>>
>>>>     ((valist + bound-1) & -bound)
>>>>
>>>> gimplifies that into an rvalue (most probably an SSA name) and uses that
>>>> to go on generating code by making valist_tmp be that returned rvalue.
>>>>
>>>> I think you'll find that removing that code will make the SSA verifier
>>>> scream or generate invalid code with -m32 when that hook is used.
>>>>
>>>
>>> Thanks for the detailed explanation. I'm not sure I understand the
>>> problem well enough, so I'll try to trigger it and investigate.
>>
>>
>> Actually the above fails to mention what the real problem is :-)  The
>> problem is that the local variable valist_tmp will be used to generate
>> further code after the above expression is generated.  Without my patch it
>> will continue to point to the VAR_DECL, not to the SSA name that actually
>> holds the computed value in the generated code.
>>
>
> I have not been able to reproduce this problem (with a bootstrap build on
> x86_64 for all languages, and {unix/,unix/-m32} testing), so I've dropped
> this bit for now.
>
> I've pushed the latest status to vries/expand-va-arg-at-pass-stdarg.
>
> -ftree-stdarg-opt (the va_list_gpr/fpr_size optimization) has been renabled
> again. I needed patch "Always check phi-ops in
> optimize_va_list_gpr_fpr_size" for that.
>
> With a similar bootstrap and reg-test as described above, there's only one
> failure left:
> ...
> FAIL: gcc.dg/tree-ssa/stdarg-2.c scan-tree-dump stdarg "f15: va_list escapes
> 0, needs to save [148] GPR units and [1-9][0-9]* FPR units"
> ...
> And this is due to the ap copy, which is classified as escape.
>
> [ We're still expanding ifn_va_arg before the va_list_gpr/fpr_size
> optimization. ]

Yeah, and the point of the exercise was of course to change that ;)

Richard.

>
> Thanks,
> - Tom