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

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

2014-09-15 19:39 GMT+04:00 Jeff Law <law@redhat.com>:
> On 09/15/14 03:51, Ilya Enkovich wrote:
>>>>
>>>> 2014-08-15  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>
>>>>          * ipa-split.c: Include tree-chkp.h.
>>>>          (find_retbnd): New.
>>>>          (consider_split): Do not split retbnd and retval
>>>>          producers.
>>>>          (split_function): Propagate Pointer Bounds Checker
>>>>          instrumentation marks and handle returned bounds.
>>>
>>>
>>> I don't think it's sufficient to just look at the SSA_NAME_DEFSTMT and
>>> verify that it's not in the header.
>>>
>>> You could easily have the SSA_NAME_DEF_STMT be a PHI which is in the same
>>> partition as the RETURN statement.  One of the PHI arguments might be fed
>>> from a statement in the header, right?
>>>
>>> Don't you have to look at the entire set of definitions which directly
>>> and
>>> indirectly feed the return statement and verify that each and every one
>>> is
>>> in the same partition as the return statement?
>>
>>
>> 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.

Ilya

>
> jeff
>