diff mbox

[Pointer,Bounds,Checker,23/x] Function split

Message ID 20140818155516.GF29976@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Aug. 18, 2014, 3:55 p.m. UTC
On 04 Jun 01:15, Jeff Law wrote:
> On 06/03/14 01:10, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch does not allow splitting in case bounds are returned until retutrned bounds are supported.  It also propagates instrumentation marks for generated call and function.
> >
> >Bootstrapped and tested on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >gcc/
> >
> >2014-06-03  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >	* ipa-split.c: Include tree-chkp.h.
> >	(consider_split): Do not split when return bounds.
> >	(split_function): Propagate Pointer Bounds Checker
> >	instrumentation marks.
> It's a hack.  There's no reason we can't support this.  So I'll
> approve on the condition that you do look at removing this
> limitation in the future.
> 
> jeff
> 

I did some work for function splitting and now patch cover more cases.  Now returned bounds are supported but it is not allowed to split producers of returned pointer and its bounds.  Is it OK?

Thanks,
Ilya
--
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.

Comments

Jeff Law Sept. 3, 2014, 7:12 p.m. UTC | #1
On 08/18/14 09:55, Ilya Enkovich wrote:
> On 04 Jun 01:15, Jeff Law wrote:
>> On 06/03/14 01:10, Ilya Enkovich wrote:
>>> Hi,
>>>
>>> This patch does not allow splitting in case bounds are returned until retutrned bounds are supported.  It also propagates instrumentation marks for generated call and function.
>>>
>>> Bootstrapped and tested on linux-x86_64.
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2014-06-03  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>> 	* ipa-split.c: Include tree-chkp.h.
>>> 	(consider_split): Do not split when return bounds.
>>> 	(split_function): Propagate Pointer Bounds Checker
>>> 	instrumentation marks.
>> It's a hack.  There's no reason we can't support this.  So I'll
>> approve on the condition that you do look at removing this
>> limitation in the future.
>>
>> jeff
>>
>
> I did some work for function splitting and now patch cover more cases.  Now returned bounds are supported but it is not allowed to split producers of returned pointer and its bounds.  Is it OK?
>
> Thanks,
> Ilya
> --
> 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?

And if so, that makes me start to think the original hack wasn't so bad 
after all :-)

jeff
Ilya Enkovich Sept. 15, 2014, 9:51 a.m. UTC | #2
2014-09-03 23:12 GMT+04:00 Jeff Law <law@redhat.com>:
> On 08/18/14 09:55, Ilya Enkovich wrote:
>>
>> On 04 Jun 01:15, Jeff Law wrote:
>>>
>>> On 06/03/14 01:10, Ilya Enkovich wrote:
>>>>
>>>> Hi,
>>>>
>>>> This patch does not allow splitting in case bounds are returned until
>>>> retutrned bounds are supported.  It also propagates instrumentation marks
>>>> for generated call and function.
>>>>
>>>> Bootstrapped and tested on linux-x86_64.
>>>>
>>>> Thanks,
>>>> Ilya
>>>> --
>>>> gcc/
>>>>
>>>> 2014-06-03  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>
>>>>         * ipa-split.c: Include tree-chkp.h.
>>>>         (consider_split): Do not split when return bounds.
>>>>         (split_function): Propagate Pointer Bounds Checker
>>>>         instrumentation marks.
>>>
>>> It's a hack.  There's no reason we can't support this.  So I'll
>>> approve on the condition that you do look at removing this
>>> limitation in the future.
>>>
>>> jeff
>>>
>>
>> I did some work for function splitting and now patch cover more cases.
>> Now returned bounds are supported but it is not allowed to split producers
>> of returned pointer and its bounds.  Is it OK?
>>
>> Thanks,
>> Ilya
>> --
>> 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.

>
> And if so, that makes me start to think the original hack wasn't so bad
> after all :-)

It's always nice to have a backup plan! :)

Ilya

>
> jeff
>
Jeff Law Sept. 15, 2014, 3:39 p.m. UTC | #3
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.

jeff
Ilya Enkovich Sept. 15, 2014, 4:20 p.m. UTC | #4
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
>
Jeff Law Sept. 15, 2014, 9:08 p.m. UTC | #5
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?

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

jeff
Ilya Enkovich Sept. 16, 2014, 9:09 a.m. UTC | #6
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
Jeff Law Sept. 19, 2014, 7:45 p.m. UTC | #7
On 09/16/14 03:09, Ilya Enkovich wrote:
>>
>> 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.
Right.  I've seen this stuff, but it's still not clear to me what the 
real issue is.

The first thing that jumps out when I look at your dump is we don't have 
  a PHI for __bound_tmp.322 in BB6.  Now it may be that we really just 
wanted __bound_tmp.322_36, but that seems wrong as the return value 
varies depending on how we reach BB6 and it seems to me the bounds ought 
to vary in a similar manner.



>
> 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.
Right, but my question is whether or not the bounds from BB2 were really 
the correct bounds to be using in the first place!  I would have 
expected a PHI in BB6 to select the bounds based on the path leading to 
BB6, much like we select a different return value.

Jeff
Ilya Enkovich Sept. 22, 2014, 6:40 a.m. UTC | #8
2014-09-19 23:45 GMT+04:00 Jeff Law <law@redhat.com>:
> On 09/16/14 03:09, Ilya Enkovich wrote:
>>>
>>>
>>> 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.
>
> Right.  I've seen this stuff, but it's still not clear to me what the real
> issue is.
>
> The first thing that jumps out when I look at your dump is we don't have  a
> PHI for __bound_tmp.322 in BB6.  Now it may be that we really just wanted
> __bound_tmp.322_36, but that seems wrong as the return value varies
> depending on how we reach BB6 and it seems to me the bounds ought to vary in
> a similar manner.

Bounds don't have to vary for different pointers.  E.g. p and p + 1
always have equal bounds.  In this particular case we have function
pointers and all of them have default bounds.

>
>
>
>>
>> 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.
>
> Right, but my question is whether or not the bounds from BB2 were really the
> correct bounds to be using in the first place!  I would have expected a PHI
> in BB6 to select the bounds based on the path leading to BB6, much like we
> select a different return value.

Consider we have pointer computation and then

return __bnd_init_ptr_bounds (res);

In such case you would never have a PHI node for bounds.  Also do not
forget that we may have no PHI nodes for both return value and return
bounds.  In such case we could also easily fall into undefined value
as in dump.

Thanks,
Ilya

>
> Jeff
Jeff Law Sept. 23, 2014, 3:55 p.m. UTC | #9
On 09/22/14 00:40, Ilya Enkovich wrote:
>
> Bounds don't have to vary for different pointers.  E.g. p and p + 1
> always have equal bounds.  In this particular case we have function
> pointers and all of them have default bounds.
OK.  It looked a bit odd and I wanted to make sure there wasn't 
something fundamentally wrong.

>>> 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.
>>
>> Right, but my question is whether or not the bounds from BB2 were really the
>> correct bounds to be using in the first place!  I would have expected a PHI
>> in BB6 to select the bounds based on the path leading to BB6, much like we
>> select a different return value.
>
> Consider we have pointer computation and then
>
> return __bnd_init_ptr_bounds (res);
>
> In such case you would never have a PHI node for bounds.  Also do not
> forget that we may have no PHI nodes for both return value and return
> bounds.  In such case we could also easily fall into undefined value
> as in dump.
This code (visit_bb, find_return_bb, consider_split) is a bit of a mess, 
but I do see what you're trying to do now.  Thanks for being patient 
with my questions.

If I were to look at this at a high level, the core issue seems to me 
that we're really not prepared to handle functions with multiple return 
values.  This shows up in your MPX work, but IIRC there's cases in the 
atomics where we have multiple return values as well.  I wouldn't be 
surprised if there's latent bugs with splitting & atomics lurking to 
bite us one day.

So if I'm reading all this code correctly, given a return block which 
returns a (pointer,bounds) pair, if the bounds are set  by a normal 
statement (ie, not a PHI), then we won't use that block for RETURN_BB. 
  So there's nothing to worry about in that case.  Similarly if the 
bounds are  set by a PHI in the return block, consider_split will reject 
that split point as well.  So really the only case here is when the 
bounds are set in another dominating block.  Right?

I can see how you're using the relevant part of the same test we need 
for the retval.  My gut tells me we want to commonize that test so that 
they don't get out-of-sync.    Specifically, can we pull the code which 
sets split_part_set_retbnd into a little function, then use it for the 
retval here too:

   else if (TREE_CODE (retval) == SSA_NAME)
     current->split_part_set_retval
       = (!SSA_NAME_IS_DEFAULT_DEF (retval)
          && (bitmap_bit_p (current->split_bbs,
                           gimple_bb (SSA_NAME_DEF_STMT (retval))->index)
              || gimple_bb (SSA_NAME_DEF_STMT (retval)) == return_bb));



Iteration through the statements in find_retbnd should start at the end 
of the block and walk backwards.  It probably doesn't matter in practice 
all that much, but might as well be sensible since the GIMPLE_RETURN is 
almost always going to be the last statement in the block.

Similarly for the statement walk in split_function when you want to 
replace retbnd with new one.

It seems like the code to build the bndret call to obtain bounds is 
repeated.  Can you refactor that into its own little function and just 
use that.  It's not a huge amount of code, but it does make things a bit 
easier to follow.

With those changes this will be OK.

Jeff
diff mbox

Patch

diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 2af3a93..f8ecaf7 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -110,6 +110,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimple-pretty-print.h"
 #include "ipa-inline.h"
 #include "cfgloop.h"
+#include "tree-chkp.h"
 
 /* Per basic block info.  */
 
@@ -151,6 +152,7 @@  struct split_point best_split_point;
 static bitmap forbidden_dominators;
 
 static tree find_retval (basic_block return_bb);
+static tree find_retbnd (basic_block return_bb);
 
 /* Callback for walk_stmt_load_store_addr_ops.  If T is non-SSA automatic
    variable, check it if it is present in bitmap passed via DATA.  */
@@ -387,6 +389,7 @@  consider_split (struct split_point *current, bitmap non_ssa_vars,
   unsigned int i;
   int incoming_freq = 0;
   tree retval;
+  tree retbnd;
   bool back_edge = false;
 
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -588,6 +591,32 @@  consider_split (struct split_point *current, bitmap non_ssa_vars,
   else
     current->split_part_set_retval = true;
 
+  /* See if retbnd used by return bb is computed by header or split part.  */
+  retbnd = find_retbnd (return_bb);
+  if (retbnd)
+    {
+      bool split_part_set_retbnd
+	= (!SSA_NAME_IS_DEFAULT_DEF (retbnd)
+	   && (bitmap_bit_p (current->split_bbs,
+			     gimple_bb (SSA_NAME_DEF_STMT (retbnd))->index)
+	       || gimple_bb (SSA_NAME_DEF_STMT (retbnd)) == return_bb));
+
+      /* If we have both return value and bounds then keep their definitions
+	 in a single function.  We use SSA names to link returned bounds and
+	 value and therefore do not handle cases when result is passed by
+	 reference (which should not be our case anyway since bounds are
+	 returned for pointers only).  */
+      if ((DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))
+	   && current->split_part_set_retval)
+	  || split_part_set_retbnd != current->split_part_set_retval)
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file,
+		     "  Refused: split point splits return value and bounds\n");
+	  return;
+	}
+    }
+
   /* split_function fixes up at most one PHI non-virtual PHI node in return_bb,
      for the return value.  If there are other PHIs, give up.  */
   if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun))
@@ -710,6 +739,18 @@  find_retval (basic_block return_bb)
   return NULL;
 }
 
+/* Given return basic block RETURN_BB, see where return bounds are really
+   stored.  */
+static tree
+find_retbnd (basic_block return_bb)
+{
+  gimple_stmt_iterator bsi;
+  for (bsi = gsi_start_bb (return_bb); !gsi_end_p (bsi); gsi_next (&bsi))
+    if (gimple_code (gsi_stmt (bsi)) == GIMPLE_RETURN)
+      return gimple_return_retbnd (gsi_stmt (bsi));
+  return NULL;
+}
+
 /* Callback for walk_stmt_load_store_addr_ops.  If T is non-SSA automatic
    variable, mark it as used in bitmap passed via DATA.
    Return true when access to T prevents splitting the function.  */
@@ -1095,8 +1136,9 @@  split_function (struct split_point *split_point)
   gimple call;
   edge e;
   edge_iterator ei;
-  tree retval = NULL, real_retval = NULL;
+  tree retval = NULL, real_retval = NULL, retbnd = NULL;
   bool split_part_return_p = false;
+  bool with_bounds = chkp_function_instrumented_p (current_function_decl);
   gimple last_stmt = NULL;
   unsigned int i;
   tree arg, ddef;
@@ -1245,6 +1287,12 @@  split_function (struct split_point *split_point)
       DECL_BUILT_IN_CLASS (node->decl) = NOT_BUILT_IN;
       DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0;
     }
+
+  /* If the original function is instrumented then it's
+     part is also instrumented.  */
+  if (with_bounds)
+    chkp_function_mark_instrumented (node->decl);
+
   /* If the original function is declared inline, there is no point in issuing
      a warning for the non-inlinable part.  */
   DECL_NO_INLINE_WARNING_P (node->decl) = 1;
@@ -1279,6 +1327,7 @@  split_function (struct split_point *split_point)
 	args_to_pass[i] = arg;
       }
   call = gimple_build_call_vec (node->decl, args_to_pass);
+  gimple_call_set_with_bounds (call, with_bounds);
   gimple_set_block (call, DECL_INITIAL (current_function_decl));
   args_to_pass.release ();
 
@@ -1385,6 +1434,7 @@  split_function (struct split_point *split_point)
       if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun))
 	{
 	  real_retval = retval = find_retval (return_bb);
+	  retbnd = find_retbnd (return_bb);
 
 	  if (real_retval && split_point->split_part_set_retval)
 	    {
@@ -1429,6 +1479,21 @@  split_function (struct split_point *split_point)
 			  }
 		      update_stmt (gsi_stmt (bsi));
 		    }
+
+		  /* Replace retbnd with new one.  */
+		  if (retbnd)
+		    {
+		      gimple_stmt_iterator bsi;
+		      for (bsi = gsi_start_bb (return_bb); !gsi_end_p (bsi);
+			   gsi_next (&bsi))
+			if (gimple_code (gsi_stmt (bsi)) == GIMPLE_RETURN)
+			  {
+			    retbnd = copy_ssa_name (retbnd, call);
+			    gimple_return_set_retbnd (gsi_stmt (bsi), retbnd);
+			    update_stmt (gsi_stmt (bsi));
+			    break;
+			  }
+		    }
 		}
 	      if (DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
 		{
@@ -1450,6 +1515,15 @@  split_function (struct split_point *split_point)
 		      gsi_insert_after (&gsi, cpy, GSI_NEW_STMT);
 		      retval = tem;
 		    }
+		  /* Build bndret call to obtain returned bounds.  */
+		  if (retbnd)
+		    {
+		      tree fndecl
+			= targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
+		      gimple bndret = gimple_build_call (fndecl, 1, retval);
+		      gimple_call_set_lhs (bndret, retbnd);
+		      gsi_insert_after (&gsi, bndret, GSI_NEW_STMT);
+		    }
 		  gimple_call_set_lhs (call, retval);
 		  update_stmt (call);
 		}
@@ -1468,6 +1542,10 @@  split_function (struct split_point *split_point)
 	    {
 	      retval = DECL_RESULT (current_function_decl);
 
+	      if (chkp_function_instrumented_p (current_function_decl)
+		  && BOUNDED_P (retval))
+		retbnd = create_tmp_reg (pointer_bounds_type_node, NULL);
+
 	      /* We use temporary register to hold value when aggregate_value_p
 		 is false.  Similarly for DECL_BY_REFERENCE we must avoid extra
 		 copy.  */
@@ -1491,6 +1569,15 @@  split_function (struct split_point *split_point)
 	        gimple_call_set_lhs (call, retval);
 	    }
           gsi_insert_after (&gsi, call, GSI_NEW_STMT);
+	  /* Build bndret call to obtain returned bounds.  */
+	  if (retbnd)
+	    {
+	      tree fndecl
+		= targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
+	      gimple bndret = gimple_build_call (fndecl, 1, retval);
+	      gimple_call_set_lhs (bndret, retbnd);
+	      gsi_insert_after (&gsi, bndret, GSI_NEW_STMT);
+	    }
 	  ret = gimple_build_return (retval);
 	  gsi_insert_after (&gsi, ret, GSI_NEW_STMT);
 	}