Re: [PATCH] tree-sra: Avoid returns of references to SRA candidates

[Martin Jambor <mjambor@suse.cz>]

Hi,

On Tue, Nov 28 2023, Jan Hubicka wrote:
>> On Tue, 28 Nov 2023, Martin Jambor wrote:
>> 
>> > On Tue, Nov 28 2023, Richard Biener wrote:
>> > > On Mon, 27 Nov 2023, Martin Jambor wrote:
>> > >
>> > >> Hi,
>> > >> 
>> > >> The enhancement to address PR 109849 contained an importsnt thinko,
>> > >> and that any reference that is passed to a function and does not
>> > >> escape, must also not happen to be aliased by the return value of the
>> > >> function.  This has quickly transpired as bugs PR 112711 and PR
>> > >> 112721.
>> > >> 
>> > >> Just as IPA-modref does a good enough job to allow us to rely on the
>> > >> escaped set of variables, it sems to be doing well also on updating
>> > >> EAF_NOT_RETURNED_DIRECTLY call argument flag which happens to address
>> > >> exactly the situation we need to avoid.  Of course, if a call
>> > >> statement ignores any returned value, we also do not need to check the
>> > >> flag.
>> > >
>> > > But what about EAF_NOT_RETURNED_INDIRECTLY?  Don't you need to
>> > > verify the parameter doesn't escape through the return at all?
>> > >
>> > 
>> > I thought EAF_NOT_RETURNED_INDIRECTLY prohibits things like "return
>> > param->next" but those are not a problem (whatever next points to cannot
>> > be an SRA candidate and any ADDR_EXPR storing its address there would
>> > trigger a disqualification or at least an assert).  But I guess I am
>> > wrong, what is actually the exact meaning of the flag?
>> 
>> I thought it's return (x.ptr = param, &x);
>> 
>> so the parameter is reachable from the return value.
>> 
>> But let's Honza answer...
> It is same difference as direct/indirect escape. so it check whether
> values pointed to by arg can be possibly returned.  Indeed maybe we
> should think of better name - the other interpretation did not even
> occur to me, but it makes sense.
>

Is my patch OK then?

(Apart from making one of the testcases x86_64-only, as Andrew pointed
out, which I wanted to do but the line somehow got lost.  Making the
testcase more general is fairly low on my contested TODO list and the
testing depends on a specific instruction trapping.)

Thanks,

Martin