diff mbox

[MPX,2/X] Pointers Checker [10/25] Calls copy and verification

Message ID 20131107115019.GJ54327@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Nov. 7, 2013, 11:50 a.m. UTC
Hi,

Here is an updated patch version.

Thanks,
Ilya
--

Comments

Jeff Law Nov. 7, 2013, 6:55 p.m. UTC | #1
On 11/07/13 04:50, Ilya Enkovich wrote:
> Hi,
>
> Here is an updated patch version.
I think this needs to hold until we have a consensus on what the 
parameter passing looks like for bounded pointers.

Jeff
Richard Biener Nov. 8, 2013, 9:43 a.m. UTC | #2
On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
> On 11/07/13 04:50, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Here is an updated patch version.
>
> I think this needs to hold until we have a consensus on what the parameter
> passing looks like for bounded pointers.

I still think the best thing to do on GIMPLE is

arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
foo (arg_2);

that is, make the parameter an implicit pair of {value, bound} where
the bound is determined by the value going through a bound association
builtin.  No extra explicit argument to the calls so arguments match
the fndecl and fntype.  All the complexity is defered to the expander
which can trivially lookup bound arguments via the SSA def (I suppose
it does that anyway now for getting at the explicit bound argument now).

As far as I can see (well, think), all currently passed bound arguments
are the return value of such builtin already.

Richard.



> Jeff
Ilya Enkovich Nov. 8, 2013, 10:03 a.m. UTC | #3
2013/11/8 Richard Biener <richard.guenther@gmail.com>:
> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>
>>> Hi,
>>>
>>> Here is an updated patch version.
>>
>> I think this needs to hold until we have a consensus on what the parameter
>> passing looks like for bounded pointers.
>
> I still think the best thing to do on GIMPLE is
>
> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
> foo (arg_2);
>
> that is, make the parameter an implicit pair of {value, bound} where
> the bound is determined by the value going through a bound association
> builtin.  No extra explicit argument to the calls so arguments match
> the fndecl and fntype.  All the complexity is defered to the expander
> which can trivially lookup bound arguments via the SSA def (I suppose
> it does that anyway now for getting at the explicit bound argument now).
>
> As far as I can see (well, think), all currently passed bound arguments
> are the return value of such builtin already.

All bounds are result of different builtin calls. Small example:

int *global_p;
void foo (int *p)
{
  int buf[10];
  bar (p, buf, global_p);
}


It is translated into:

  __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
  __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
  global_p.0_2 = global_p;
  __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
  bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
global_p.0_2, __bound_tmp.1_8);

Bounds binding via calls as you suggest may be done as following:

  __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
  __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
  global_p.0_2 = global_p;
  __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
  _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
  _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
  _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
  bar (_9, _10, _11);

Is it close to what you propose?

Ilya
>
> Richard.
>
>
>
>> Jeff
Jakub Jelinek Nov. 8, 2013, 10:07 a.m. UTC | #4
On Fri, Nov 08, 2013 at 10:43:26AM +0100, Richard Biener wrote:
> >> Here is an updated patch version.
> >
> > I think this needs to hold until we have a consensus on what the parameter
> > passing looks like for bounded pointers.
> 
> I still think the best thing to do on GIMPLE is
> 
> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
> foo (arg_2);

Well, in that case it would likely have to be an internal builtin fn,
because it needs to be a pass through for random argument types, so
return long for long argument, or double for double argument, etc.
Or is that only done for pointer arguments?  Then it could be
like __builtin_assume_aligned, which takes void * and returns void *.

	Jakub
Richard Biener Nov. 11, 2013, 1:46 p.m. UTC | #5
On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>
>>>> Hi,
>>>>
>>>> Here is an updated patch version.
>>>
>>> I think this needs to hold until we have a consensus on what the parameter
>>> passing looks like for bounded pointers.
>>
>> I still think the best thing to do on GIMPLE is
>>
>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>> foo (arg_2);
>>
>> that is, make the parameter an implicit pair of {value, bound} where
>> the bound is determined by the value going through a bound association
>> builtin.  No extra explicit argument to the calls so arguments match
>> the fndecl and fntype.  All the complexity is defered to the expander
>> which can trivially lookup bound arguments via the SSA def (I suppose
>> it does that anyway now for getting at the explicit bound argument now).
>>
>> As far as I can see (well, think), all currently passed bound arguments
>> are the return value of such builtin already.
>
> All bounds are result of different builtin calls. Small example:
>
> int *global_p;
> void foo (int *p)
> {
>   int buf[10];
>   bar (p, buf, global_p);
> }
>
>
> It is translated into:
>
>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>   global_p.0_2 = global_p;
>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
> global_p.0_2, __bound_tmp.1_8);
>
> Bounds binding via calls as you suggest may be done as following:
>
>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>   global_p.0_2 = global_p;
>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>   bar (_9, _10, _11);
>
> Is it close to what you propose?

Yes.

Richard.

> Ilya
>>
>> Richard.
>>
>>
>>
>>> Jeff
Ilya Enkovich Nov. 11, 2013, 2 p.m. UTC | #6
2013/11/11 Richard Biener <richard.guenther@gmail.com>:
> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Here is an updated patch version.
>>>>
>>>> I think this needs to hold until we have a consensus on what the parameter
>>>> passing looks like for bounded pointers.
>>>
>>> I still think the best thing to do on GIMPLE is
>>>
>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>> foo (arg_2);
>>>
>>> that is, make the parameter an implicit pair of {value, bound} where
>>> the bound is determined by the value going through a bound association
>>> builtin.  No extra explicit argument to the calls so arguments match
>>> the fndecl and fntype.  All the complexity is defered to the expander
>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>> it does that anyway now for getting at the explicit bound argument now).
>>>
>>> As far as I can see (well, think), all currently passed bound arguments
>>> are the return value of such builtin already.
>>
>> All bounds are result of different builtin calls. Small example:
>>
>> int *global_p;
>> void foo (int *p)
>> {
>>   int buf[10];
>>   bar (p, buf, global_p);
>> }
>>
>>
>> It is translated into:
>>
>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>   global_p.0_2 = global_p;
>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>> global_p.0_2, __bound_tmp.1_8);
>>
>> Bounds binding via calls as you suggest may be done as following:
>>
>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>   global_p.0_2 = global_p;
>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>   bar (_9, _10, _11);
>>
>> Is it close to what you propose?
>
> Yes.

Is there a way to bind bounds with structures in a similar way?  For
SSA names I may easily find definition and check if it is a binding
builtin call. But for structures it is not possible.  The way I see it
to mark all such args as addressable and load required bounds on
expand pass.

Ilya
>
> Richard.
>
>> Ilya
>>>
>>> Richard.
>>>
>>>
>>>
>>>> Jeff
Richard Biener Nov. 11, 2013, 2:16 p.m. UTC | #7
On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Here is an updated patch version.
>>>>>
>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>> passing looks like for bounded pointers.
>>>>
>>>> I still think the best thing to do on GIMPLE is
>>>>
>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>> foo (arg_2);
>>>>
>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>> the bound is determined by the value going through a bound association
>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>
>>>> As far as I can see (well, think), all currently passed bound arguments
>>>> are the return value of such builtin already.
>>>
>>> All bounds are result of different builtin calls. Small example:
>>>
>>> int *global_p;
>>> void foo (int *p)
>>> {
>>>   int buf[10];
>>>   bar (p, buf, global_p);
>>> }
>>>
>>>
>>> It is translated into:
>>>
>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>   global_p.0_2 = global_p;
>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>> global_p.0_2, __bound_tmp.1_8);
>>>
>>> Bounds binding via calls as you suggest may be done as following:
>>>
>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>   global_p.0_2 = global_p;
>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>   bar (_9, _10, _11);
>>>
>>> Is it close to what you propose?
>>
>> Yes.
>
> Is there a way to bind bounds with structures in a similar way?

Not to have them easy to lookup in the SSA web.  A long time ago
I proposed to make SSA aggregates possible, so you could do

 tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
__bound_tmp.1_4, ...);
 bar (tem_2);

(originally the SSA aggregates were supposed to make copy-propgagation
possible using the SSA copy propagator, and of course I needed it for
the middle-end array work)

Not sure if that will give other people the creeps (expand would
never expand the "load" from tem_2 but instead handle aggr as
parameter).  A slight complication is due to alias analysis
which would be need to taught that bar performs a load of aggr.

Richard.

>  For
> SSA names I may easily find definition and check if it is a binding
> builtin call. But for structures it is not possible.  The way I see it
> to mark all such args as addressable and load required bounds on
> expand pass.
>
> Ilya
>>
>> Richard.
>>
>>> Ilya
>>>>
>>>> Richard.
>>>>
>>>>
>>>>
>>>>> Jeff
Ilya Enkovich Nov. 11, 2013, 2:45 p.m. UTC | #8
2013/11/11 Richard Biener <richard.guenther@gmail.com>:
> On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Here is an updated patch version.
>>>>>>
>>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>>> passing looks like for bounded pointers.
>>>>>
>>>>> I still think the best thing to do on GIMPLE is
>>>>>
>>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>>> foo (arg_2);
>>>>>
>>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>>> the bound is determined by the value going through a bound association
>>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>>
>>>>> As far as I can see (well, think), all currently passed bound arguments
>>>>> are the return value of such builtin already.
>>>>
>>>> All bounds are result of different builtin calls. Small example:
>>>>
>>>> int *global_p;
>>>> void foo (int *p)
>>>> {
>>>>   int buf[10];
>>>>   bar (p, buf, global_p);
>>>> }
>>>>
>>>>
>>>> It is translated into:
>>>>
>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>   global_p.0_2 = global_p;
>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>>> global_p.0_2, __bound_tmp.1_8);
>>>>
>>>> Bounds binding via calls as you suggest may be done as following:
>>>>
>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>   global_p.0_2 = global_p;
>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>>   bar (_9, _10, _11);
>>>>
>>>> Is it close to what you propose?
>>>
>>> Yes.
>>
>> Is there a way to bind bounds with structures in a similar way?
>
> Not to have them easy to lookup in the SSA web.  A long time ago
> I proposed to make SSA aggregates possible, so you could do
>
>  tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
> __bound_tmp.1_4, ...);
>  bar (tem_2);
>
> (originally the SSA aggregates were supposed to make copy-propgagation
> possible using the SSA copy propagator, and of course I needed it for
> the middle-end array work)
>
> Not sure if that will give other people the creeps (expand would
> never expand the "load" from tem_2 but instead handle aggr as
> parameter).  A slight complication is due to alias analysis
> which would be need to taught that bar performs a load of aggr.

It would require bounds loading for aggr before __internal_bind_bounds
anyway.  So, why not to do it in expand?  I just need to mark calls
with a flag (which you've proposed few times already) to let expand
know when it should load bounds.  Having SSA aggregates would be nice
but I suspect it has much higher impact then loading bounds in expand.
 I want to try simpler variant first.

Thanks,
Ilya

>
> Richard.
>
>>  For
>> SSA names I may easily find definition and check if it is a binding
>> builtin call. But for structures it is not possible.  The way I see it
>> to mark all such args as addressable and load required bounds on
>> expand pass.
>>
>> Ilya
>>>
>>> Richard.
>>>
>>>> Ilya
>>>>>
>>>>> Richard.
>>>>>
>>>>>
>>>>>
>>>>>> Jeff
Richard Biener Nov. 11, 2013, 2:48 p.m. UTC | #9
On Mon, Nov 11, 2013 at 3:45 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>> On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>>>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Here is an updated patch version.
>>>>>>>
>>>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>>>> passing looks like for bounded pointers.
>>>>>>
>>>>>> I still think the best thing to do on GIMPLE is
>>>>>>
>>>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>>>> foo (arg_2);
>>>>>>
>>>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>>>> the bound is determined by the value going through a bound association
>>>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>>>
>>>>>> As far as I can see (well, think), all currently passed bound arguments
>>>>>> are the return value of such builtin already.
>>>>>
>>>>> All bounds are result of different builtin calls. Small example:
>>>>>
>>>>> int *global_p;
>>>>> void foo (int *p)
>>>>> {
>>>>>   int buf[10];
>>>>>   bar (p, buf, global_p);
>>>>> }
>>>>>
>>>>>
>>>>> It is translated into:
>>>>>
>>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>>   global_p.0_2 = global_p;
>>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>>>> global_p.0_2, __bound_tmp.1_8);
>>>>>
>>>>> Bounds binding via calls as you suggest may be done as following:
>>>>>
>>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>>   global_p.0_2 = global_p;
>>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>>>   bar (_9, _10, _11);
>>>>>
>>>>> Is it close to what you propose?
>>>>
>>>> Yes.
>>>
>>> Is there a way to bind bounds with structures in a similar way?
>>
>> Not to have them easy to lookup in the SSA web.  A long time ago
>> I proposed to make SSA aggregates possible, so you could do
>>
>>  tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
>> __bound_tmp.1_4, ...);
>>  bar (tem_2);
>>
>> (originally the SSA aggregates were supposed to make copy-propgagation
>> possible using the SSA copy propagator, and of course I needed it for
>> the middle-end array work)
>>
>> Not sure if that will give other people the creeps (expand would
>> never expand the "load" from tem_2 but instead handle aggr as
>> parameter).  A slight complication is due to alias analysis
>> which would be need to taught that bar performs a load of aggr.
>
> It would require bounds loading for aggr before __internal_bind_bounds
> anyway.  So, why not to do it in expand?

Ah, ok I wasn't aware of that.

>  I just need to mark calls
> with a flag (which you've proposed few times already) to let expand
> know when it should load bounds.  Having SSA aggregates would be nice
> but I suspect it has much higher impact then loading bounds in expand.
>  I want to try simpler variant first.

Good.

Richard.

> Thanks,
> Ilya
>
>>
>> Richard.
>>
>>>  For
>>> SSA names I may easily find definition and check if it is a binding
>>> builtin call. But for structures it is not possible.  The way I see it
>>> to mark all such args as addressable and load required bounds on
>>> expand pass.
>>>
>>> Ilya
>>>>
>>>> Richard.
>>>>
>>>>> Ilya
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Jeff
Ilya Enkovich Nov. 15, 2013, 5:59 p.m. UTC | #10
2013/11/7 Jeff Law <law@redhat.com>:
> On 11/07/13 04:50, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Here is an updated patch version.
>
> I think this needs to hold until we have a consensus on what the parameter
> passing looks like for bounded pointers.

With the new parameters passing model this patch is not needed.

Thanks,
Ilya

>
> Jeff
diff mbox

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 52d9ab0..9d7ae85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3030,40 +3030,54 @@  gimple_check_call_args (gimple stmt, tree fndecl, bool args_count_match)
     {
       for (i = 0, p = DECL_ARGUMENTS (fndecl);
 	   i < nargs;
-	   i++, p = DECL_CHAIN (p))
+	   i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* We cannot distinguish a varargs function from the case
 	     of excess parameters, still deferring the inlining decision
 	     to the callee is possible.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (p == error_mark_node
 	      || arg == error_mark_node
 	      || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
             return false;
+
+	  p = DECL_CHAIN (p);
 	}
       if (args_count_match && p)
 	return false;
     }
   else if (parms)
     {
-      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+      for (i = 0, p = parms; i < nargs; i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* If this is a varargs function defer inlining decision
 	     to callee.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (TREE_VALUE (p) == error_mark_node
 	      || arg == error_mark_node
 	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
 	      || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (TREE_VALUE (p), arg)))
             return false;
+
+	  p = TREE_CHAIN (p);
 	}
     }
   else
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 20f6010..0fc704b 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -3043,20 +3043,27 @@  canonicalize_cond_expr_cond (tree t)
 }
 
 /* Build a GIMPLE_CALL identical to STMT but skipping the arguments in
-   the positions marked by the set ARGS_TO_SKIP.  */
+   the positions marked by the set ARGS_TO_SKIP.  ARGS_TO_SKIP does not
+   take into account bounds arguments.  Bounds passed for skipped args
+   are also skipped.  */
 
 gimple
 gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
 {
-  int i;
+  int i, bit;
   int nargs = gimple_call_num_args (stmt);
   vec<tree> vargs;
   vargs.create (nargs);
   gimple new_stmt;
 
-  for (i = 0; i < nargs; i++)
-    if (!bitmap_bit_p (args_to_skip, i))
-      vargs.quick_push (gimple_call_arg (stmt, i));
+  for (i = 0, bit = 0; i < nargs; i++, bit++)
+      if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
+	{
+	  if (!bitmap_bit_p (args_to_skip, --bit))
+	    vargs.quick_push (gimple_call_arg (stmt, i));
+	}
+      else if (!bitmap_bit_p (args_to_skip, bit))
+	  vargs.quick_push (gimple_call_arg (stmt, i));
 
   if (gimple_call_internal_p (stmt))
     new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn (stmt),
@@ -3702,6 +3709,9 @@  validate_call (gimple stmt, tree fndecl)
       if (!targs)
 	return true;
       tree arg = gimple_call_arg (stmt, i);
+      /* Skip bounds.  */
+      if (POINTER_BOUNDS_P (arg))
+	continue;
       if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
 	  && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
 	;