Re: [PATCH, Pointer Bounds Checker 23/x] Function split

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2014-09-16 1:08 GMT+04:00 Jeff Law <law@redhat.com>:
> On 09/15/14 10:20, Ilya Enkovich wrote:
>>>>
>>>>
>>>> A problem I'm trying to avoid is that bounds in return statement are
>>>> not taken into account when checking for data dependencies between
>>>> parts.  It means we may have a case when return statement with bounds
>>>> is put into split part but bounds producer is not.  If
>>>> SSA_NAME_DEFSTMT for returned bounds is in the same partition as a
>>>> return then I do not think I should care about the rest of definitions
>>>> chain because regular split point checks should make sure we have
>>>> everything required.
>>>
>>>
>>> Is the data dependency in the gimple IL?  If so there shouldn't be
>>> anything
>>> particularly special we need to do.  If not, then how ugly would it be to
>>> "use" the bounds at the return statement to expose the missing
>>> dependency?
>>>
>>> Not asking you to make that change, just want to make sure that I
>>> understand
>>> the core issue and that if something is missing from a dependency
>>> standpoint
>>> that we consider what it would take to expose the missing dependency.
>>
>>
>> Gimple IL has required data dependencies to handle returns properly.
>> But split pass handles return basic block in a special way.  Return
>> basic block has to have a simple form and is not scanned using stmt
>> walkers as it is done for all other BBs by visit_bb.  It is assumed
>> that all dependencies for return BB are PHI args and returned value.
>> Thus returned bounds are just not taken into account.  That's how I
>> see the problem.
>
> I must be misunderstanding something then.  I fundamentally don't see how
> the return bounds are any different here than the return value.  If we have
> exposed the bounds in the IL, then aren't they going to be handled just like
> any other object in the IL?

They are not handled like any other object in IL because return block
and all statements in it are not handled as all other statements we
put into split part.

Here is a comment from find_return_bb:

/* Return basic block containing RETURN statement.  We allow basic blocks
   of the form:
   <retval> = tmp_var;
   return <retval>
   but return_bb can not be more complex than this.
...
*/

Phi nodes also may present in return_bb.

All blocks going to split part are analyzed by visit_bb function.
Return basic block is not analyzed in the same way but still may be
copied into split part in case return value is defined in it.  There
is a special code in visit_bb to add args of phi statements of
return_bb as uses of split part to have no undefined values in copied
block.  It was enough when those phi args plus return value were only
uses in return_bb.

But now we add returned bounds to GIMPLE_RETURN as a new use and this
new use is ignored.  If split part returns value then return_bb will
be copied into it.  It means I should check returned bounds are
defined there too.  If SSA_NAME_DEF_STMT of returned bounds is in
split part then it is OK.  If SSA_NAME_DEF_STMT of returned bounds is
in return_bb then it is also OK because it means it is a result of PHI
node whose args were added as additional uses for split part earlier
in visit_bb.

At least that is how I think this happens :)

>
> Maybe you should post the IL for a case where this all matters and walk me
> through the key issues.

I attach a dump I got from Chrome compilation with no additional
checks restrictions in split.  Original function returns value defined
by phi node in return_bb and bounds defined in BB2.  Split part
contains BB3, BB4 and BB5 and resulting function part has usage of
returned bounds but no producer for it.

Thanks,
Ilya

>
> jeff

Attachment: split.dump
Description: Binary data