Re: [tree-tailcall] Check if function returns it's argument

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 2 December 2016 at 03:57, Jeff Law <law@redhat.com> wrote:
> On 12/01/2016 06:22 AM, Richard Biener wrote:
>>>
>>> Well after removing DECL_BY_REFERENCE, verify_gimple still fails but
>>> differently:
>>>
>>> tail-padding1.C:13:1: error: RESULT_DECL should be read only when
>>> DECL_BY_REFERENCE is set
>>>  }
>>>  ^
>>> while verifying SSA_NAME nrvo_4 in statement
>>> # .MEM_3 = VDEF <.MEM_1(D)>
>>>     nrvo_4 = __builtin_memset (nrvo_2(D), 0, 8);
>>> tail-padding1.C:13:1: internal compiler error: verify_ssa failed
>>
>>
>> Hmm, ok.  Not sure why we enforce this.
>
> I don't know either.  But I would start by looking at tree-nrv.c since it
> looks (based on the variable names) that the named-value-return optimization
> kicked in.
Um, the name nrv0 was in the test-case itself. The transform takes
place in tailr1 pass,
which appears to be before nrv, so possibly this is not related to nrv ?

The verify check seems to be added in r161898 by Honza to fix PR 44813
based on Richard's following suggestion from
https://gcc.gnu.org/ml/gcc-patches/2010-07/msg00358.html:

"We should never see a defintion of a RESULT_DECL SSA name for
DECL_BY_REFERENCE RESULT_DECLs (that would
be a bug - we should add verification to the SSA verifier, can you
do add that?)."

The attached patch moves && ret_stmt together with !ass_var,
and keeps the !DECL_BY_REFERENCE (DECL_RESULT (cfun->decl))
check, and adjusts tailcall-9.c testcase to scan _\[0-9\]* = __builtin_memcpy
in tailr1 dump since that's where the transform takes place.
Is this version OK ?

Thanks,
Prathamesh
>
>>
>> Note that in the end this patch looks fishy -- iff we really need
>> the LHS on the assignment for correctness if we have the tailcall
>> flag set then what guarantees that later passes do not remove it
>> again?  So anybody removing a LHS would need to unset the tailcall flag?
>>
>> Saying again that I don't know enough about the RTL part of tailcall
>> expansion.
>
> The LHS on the assignment makes it easier to identify when a tail call is
> possible.  It's not needed for correctness.  Not having the LHS on the
> assignment just means we won't get an optimized tail call.
>
> Under what circumstances would the LHS possibly be removed?  We know the
> return statement references the LHS, so it's not going to be something that
> DCE will do.
>
> jeff

Attachment: tailcall-8.diff
Description: Text document