[MPX,2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.

Hi,

Here is a patch which hadles the problem with optimization of BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that argument of this call is a default SSA_NAME of the PARM_DECL whose bounds we want to get.  The problem is in optimizations which may replace arg with it's copy or a known value.  This patch suppress such modifications.

Thanks,
Ilya
--

gcc/

2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-into-ssa.c: Include "target.h"
	(rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
	* tree-ssa-dom.c: Include "target.h"
	(cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.

On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> Here is a patch which hadles the problem with optimization of BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that argument of this call is a default SSA_NAME of the PARM_DECL whose bounds we want to get.  The problem is in optimizations which may replace arg with it's copy or a known value.  This patch suppress such modifications.

This doesn't seem like a good fix.  I suppose you require the same on
RTL, that is, have the incoming arg reg coalesced with the use?
In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.

Richard.

> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * tree-into-ssa.c: Include "target.h"
>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>         * tree-ssa-dom.c: Include "target.h"
>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>
>
> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
> index 981e9f4..8d48f6d 100644
> --- a/gcc/tree-into-ssa.c
> +++ b/gcc/tree-into-ssa.c
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "params.h"
>  #include "diagnostic-core.h"
>  #include "tree-into-ssa.h"
> +#include "target.h"
>
>
>  /* This file builds the SSA form for a function as described in:
> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt, gimple_stmt_iterator gsi)
>      }
>
>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose underlying
> -     symbol is marked for renaming.  */
> -  if (rewrite_uses_p (stmt))
> +     symbol is marked for renaming.
> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
> +     renamed.  */
> +  if (rewrite_uses_p (stmt)
> +      && !(flag_check_pointer_bounds
> +          && (gimple_code (stmt) == GIMPLE_CALL)
> +          && gimple_call_fndecl (stmt)
> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>      {
>        if (is_gimple_debug (stmt))
>         {
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index 211bfcf..445278a 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "params.h"
>  #include "tree-ssa-threadedge.h"
>  #include "tree-ssa-dom.h"
> +#include "target.h"
>
>  /* This file implements optimizations on the dominator tree.  */
>
> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>    use_operand_p op_p;
>    ssa_op_iter iter;
>
> +  /* Call used to obtain bounds of input arg by Pointer Bounds Checker
> +     should not be optimized.  Argument of the call is a default
> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.  */
> +  if (flag_check_pointer_bounds && gimple_code (stmt) == GIMPLE_CALL)
> +    {
> +      tree fndecl = gimple_call_fndecl (stmt);
> +      if (fndecl == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND))
> +       return;
> +    }
> +
>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>      cprop_operand (stmt, op_p);
>  }

Richard Biener <richard.guenther@gmail.com> wrote:
>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
><enkovich.gnu@gmail.com> wrote:
>> Hi,
>>
>> Here is a patch which hadles the problem with optimization of
>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>argument of this call is a default SSA_NAME of the PARM_DECL whose
>bounds we want to get.  The problem is in optimizations which may
>replace arg with it's copy or a known value.  This patch suppress such
>modifications.
>
>This doesn't seem like a good fix.  I suppose you require the same on
>RTL, that is, have the incoming arg reg coalesced with the use?
>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.

Btw, I would have chosen

P_2 = __builtin_xyz (p_1, bound)
Call (p_2)

Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue

Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?

Richard.


>Richard.
>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * tree-into-ssa.c: Include "target.h"
>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>         * tree-ssa-dom.c: Include "target.h"
>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>
>>
>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>> index 981e9f4..8d48f6d 100644
>> --- a/gcc/tree-into-ssa.c
>> +++ b/gcc/tree-into-ssa.c
>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "params.h"
>>  #include "diagnostic-core.h"
>>  #include "tree-into-ssa.h"
>> +#include "target.h"
>>
>>
>>  /* This file builds the SSA form for a function as described in:
>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>gimple_stmt_iterator gsi)
>>      }
>>
>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>underlying
>> -     symbol is marked for renaming.  */
>> -  if (rewrite_uses_p (stmt))
>> +     symbol is marked for renaming.
>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>> +     renamed.  */
>> +  if (rewrite_uses_p (stmt)
>> +      && !(flag_check_pointer_bounds
>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>> +          && gimple_call_fndecl (stmt)
>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>      {
>>        if (is_gimple_debug (stmt))
>>         {
>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>> index 211bfcf..445278a 100644
>> --- a/gcc/tree-ssa-dom.c
>> +++ b/gcc/tree-ssa-dom.c
>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "params.h"
>>  #include "tree-ssa-threadedge.h"
>>  #include "tree-ssa-dom.h"
>> +#include "target.h"
>>
>>  /* This file implements optimizations on the dominator tree.  */
>>
>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>    use_operand_p op_p;
>>    ssa_op_iter iter;
>>
>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>Checker
>> +     should not be optimized.  Argument of the call is a default
>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value. 
>*/
>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>GIMPLE_CALL)
>> +    {
>> +      tree fndecl = gimple_call_fndecl (stmt);
>> +      if (fndecl == targetm.builtin_chkp_function
>(BUILT_IN_CHKP_ARG_BND))
>> +       return;
>> +    }
>> +
>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>      cprop_operand (stmt, op_p);
>>  }

2013/11/4 Richard Biener <richard.guenther@gmail.com>:
> Richard Biener <richard.guenther@gmail.com> wrote:
>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>><enkovich.gnu@gmail.com> wrote:
>>> Hi,
>>>
>>> Here is a patch which hadles the problem with optimization of
>>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>bounds we want to get.  The problem is in optimizations which may
>>replace arg with it's copy or a known value.  This patch suppress such
>>modifications.
>>
>>This doesn't seem like a good fix.  I suppose you require the same on
>>RTL, that is, have the incoming arg reg coalesced with the use?
>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>
> Btw, I would have chosen
>
> P_2 = __builtin_xyz (p_1, bound)
> Call (p_2)
>
> Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue
>
> Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?

In RTL all incoming bounds are materialized into slot where bounds are
passed and arg_bnd call is expanded into this slot.  Thus in RTL bound
arg looks more like a regular arg.

If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
because it does not used in abnormal phi, shouldn't it?  Also it would
prevent all optimizations for these SSA_NAMEs right?  Instrumentation
is performed on the earlier stage, right after we build SSA. I think
using abnormal phi flag and binding pointers with bounds via calls
would prevent some useful optimizations.

Many interprocedural optimizations require some support when work with
instrumented calls.  Inlining support includes:
  - replacement of arg_bnd calls with actual bounds passed to the inlined call
  - replacement of retbnd call with bounds, returned by inlined function

Not all IPA passes are fully enabled right now.  E.g. I restrict
bounded value propagation in ipa-prop and bounded args in functions
generated by ipa-split.  Such features will be enabled later.

For nested functions I do not see much difference from checker point
of view.  It just has an additional static chain param. Probably I
miss here something.  I did just few tests with nested functions.

Ilya

>
> Richard.
>
>
>>Richard.
>>
>>> Thanks,
>>> Ilya
>>> --
>>>
>>> gcc/
>>>
>>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>         * tree-into-ssa.c: Include "target.h"
>>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>         * tree-ssa-dom.c: Include "target.h"
>>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>
>>>
>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>> index 981e9f4..8d48f6d 100644
>>> --- a/gcc/tree-into-ssa.c
>>> +++ b/gcc/tree-into-ssa.c
>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "params.h"
>>>  #include "diagnostic-core.h"
>>>  #include "tree-into-ssa.h"
>>> +#include "target.h"
>>>
>>>
>>>  /* This file builds the SSA form for a function as described in:
>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>gimple_stmt_iterator gsi)
>>>      }
>>>
>>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>underlying
>>> -     symbol is marked for renaming.  */
>>> -  if (rewrite_uses_p (stmt))
>>> +     symbol is marked for renaming.
>>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>> +     renamed.  */
>>> +  if (rewrite_uses_p (stmt)
>>> +      && !(flag_check_pointer_bounds
>>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>>> +          && gimple_call_fndecl (stmt)
>>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>>      {
>>>        if (is_gimple_debug (stmt))
>>>         {
>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>> index 211bfcf..445278a 100644
>>> --- a/gcc/tree-ssa-dom.c
>>> +++ b/gcc/tree-ssa-dom.c
>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "params.h"
>>>  #include "tree-ssa-threadedge.h"
>>>  #include "tree-ssa-dom.h"
>>> +#include "target.h"
>>>
>>>  /* This file implements optimizations on the dominator tree.  */
>>>
>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>    use_operand_p op_p;
>>>    ssa_op_iter iter;
>>>
>>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>>Checker
>>> +     should not be optimized.  Argument of the call is a default
>>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.
>>*/
>>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>GIMPLE_CALL)
>>> +    {
>>> +      tree fndecl = gimple_call_fndecl (stmt);
>>> +      if (fndecl == targetm.builtin_chkp_function
>>(BUILT_IN_CHKP_ARG_BND))
>>> +       return;
>>> +    }
>>> +
>>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>>      cprop_operand (stmt, op_p);
>>>  }
>
>

On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/4 Richard Biener <richard.guenther@gmail.com>:
>> Richard Biener <richard.guenther@gmail.com> wrote:
>>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>>><enkovich.gnu@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> Here is a patch which hadles the problem with optimization of
>>>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>>bounds we want to get.  The problem is in optimizations which may
>>>replace arg with it's copy or a known value.  This patch suppress such
>>>modifications.
>>>
>>>This doesn't seem like a good fix.  I suppose you require the same on
>>>RTL, that is, have the incoming arg reg coalesced with the use?
>>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>
>> Btw, I would have chosen
>>
>> P_2 = __builtin_xyz (p_1, bound)
>> Call (p_2)
>>
>> Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue
>>
>> Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?
>
> In RTL all incoming bounds are materialized into slot where bounds are
> passed and arg_bnd call is expanded into this slot.  Thus in RTL bound
> arg looks more like a regular arg.

I don't care so much for RTL, but this description hints at that the
suggestion above would work (and it would eliminate all my concerns about
the representation on the GIMPLE level - you'd not even need this
strange POINTER_BOUNDS_TYPE as far as I can see.

> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
> because it does not used in abnormal phi, shouldn't it?  Also it would
> prevent all optimizations for these SSA_NAMEs right?  Instrumentation
> is performed on the earlier stage, right after we build SSA. I think
> using abnormal phi flag and binding pointers with bounds via calls
> would prevent some useful optimizations.

Well, what are the constraints that you need to avoid propagation in
the first place?

> Many interprocedural optimizations require some support when work with
> instrumented calls.  Inlining support includes:
>   - replacement of arg_bnd calls with actual bounds passed to the inlined call
>   - replacement of retbnd call with bounds, returned by inlined function
>
> Not all IPA passes are fully enabled right now.  E.g. I restrict
> bounded value propagation in ipa-prop and bounded args in functions
> generated by ipa-split.  Such features will be enabled later.
>
> For nested functions I do not see much difference from checker point
> of view.  It just has an additional static chain param. Probably I
> miss here something.  I did just few tests with nested functions.

I was thinking of

int foo (char *s)
{
   int bar (void)
   {
      ... use bound of 's' of the containing function ...
      foo (q, and pass it along here for q)
   }
}

that is references to the containing functions parameters and their bounds.

Richard.

> Ilya
>
>>
>> Richard.
>>
>>
>>>Richard.
>>>
>>>> Thanks,
>>>> Ilya
>>>> --
>>>>
>>>> gcc/
>>>>
>>>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>
>>>>         * tree-into-ssa.c: Include "target.h"
>>>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>         * tree-ssa-dom.c: Include "target.h"
>>>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>
>>>>
>>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>>> index 981e9f4..8d48f6d 100644
>>>> --- a/gcc/tree-into-ssa.c
>>>> +++ b/gcc/tree-into-ssa.c
>>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>  #include "params.h"
>>>>  #include "diagnostic-core.h"
>>>>  #include "tree-into-ssa.h"
>>>> +#include "target.h"
>>>>
>>>>
>>>>  /* This file builds the SSA form for a function as described in:
>>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>>gimple_stmt_iterator gsi)
>>>>      }
>>>>
>>>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>>underlying
>>>> -     symbol is marked for renaming.  */
>>>> -  if (rewrite_uses_p (stmt))
>>>> +     symbol is marked for renaming.
>>>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>>> +     renamed.  */
>>>> +  if (rewrite_uses_p (stmt)
>>>> +      && !(flag_check_pointer_bounds
>>>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>>>> +          && gimple_call_fndecl (stmt)
>>>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>>>      {
>>>>        if (is_gimple_debug (stmt))
>>>>         {
>>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>>> index 211bfcf..445278a 100644
>>>> --- a/gcc/tree-ssa-dom.c
>>>> +++ b/gcc/tree-ssa-dom.c
>>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>  #include "params.h"
>>>>  #include "tree-ssa-threadedge.h"
>>>>  #include "tree-ssa-dom.h"
>>>> +#include "target.h"
>>>>
>>>>  /* This file implements optimizations on the dominator tree.  */
>>>>
>>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>>    use_operand_p op_p;
>>>>    ssa_op_iter iter;
>>>>
>>>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>>>Checker
>>>> +     should not be optimized.  Argument of the call is a default
>>>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.
>>>*/
>>>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>>GIMPLE_CALL)
>>>> +    {
>>>> +      tree fndecl = gimple_call_fndecl (stmt);
>>>> +      if (fndecl == targetm.builtin_chkp_function
>>>(BUILT_IN_CHKP_ARG_BND))
>>>> +       return;
>>>> +    }
>>>> +
>>>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>>>      cprop_operand (stmt, op_p);
>>>>  }
>>
>>

2013/11/5 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/4 Richard Biener <richard.guenther@gmail.com>:
>>> Richard Biener <richard.guenther@gmail.com> wrote:
>>>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>>>><enkovich.gnu@gmail.com> wrote:
>>>>> Hi,
>>>>>
>>>>> Here is a patch which hadles the problem with optimization of
>>>>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>>>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>>>bounds we want to get.  The problem is in optimizations which may
>>>>replace arg with it's copy or a known value.  This patch suppress such
>>>>modifications.
>>>>
>>>>This doesn't seem like a good fix.  I suppose you require the same on
>>>>RTL, that is, have the incoming arg reg coalesced with the use?
>>>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>>
>>> Btw, I would have chosen
>>>
>>> P_2 = __builtin_xyz (p_1, bound)
>>> Call (p_2)
>>>
>>> Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue
>>>
>>> Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?
>>
>> In RTL all incoming bounds are materialized into slot where bounds are
>> passed and arg_bnd call is expanded into this slot.  Thus in RTL bound
>> arg looks more like a regular arg.
>
> I don't care so much for RTL, but this description hints at that the
> suggestion above would work (and it would eliminate all my concerns about
> the representation on the GIMPLE level - you'd not even need this
> strange POINTER_BOUNDS_TYPE as far as I can see.
>
>> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
>> because it does not used in abnormal phi, shouldn't it?  Also it would
>> prevent all optimizations for these SSA_NAMEs right?  Instrumentation
>> is performed on the earlier stage, right after we build SSA. I think
>> using abnormal phi flag and binding pointers with bounds via calls
>> would prevent some useful optimizations.
>
> Well, what are the constraints that you need to avoid propagation in
> the first place?

For input parameter P I need to have
  BOUNDS = __builtin_arg_bnd (P)
to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
__builtin_arg_bnd (P) replacing P with its copy or some value. It
makes call useless because removes information about parameter whose
bounds we refer to. I want such optimization to ignore
__builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
there as arg.

>
>> Many interprocedural optimizations require some support when work with
>> instrumented calls.  Inlining support includes:
>>   - replacement of arg_bnd calls with actual bounds passed to the inlined call
>>   - replacement of retbnd call with bounds, returned by inlined function
>>
>> Not all IPA passes are fully enabled right now.  E.g. I restrict
>> bounded value propagation in ipa-prop and bounded args in functions
>> generated by ipa-split.  Such features will be enabled later.
>>
>> For nested functions I do not see much difference from checker point
>> of view.  It just has an additional static chain param. Probably I
>> miss here something.  I did just few tests with nested functions.
>
> I was thinking of
>
> int foo (char *s)
> {
>    int bar (void)
>    {
>       ... use bound of 's' of the containing function ...
>       foo (q, and pass it along here for q)
>    }
> }
>
> that is references to the containing functions parameters and their

All foo's locals referenced by nested bar here are placed into special
structure and all bounds should be stored in appropriated entries of
Bound Table.  Nested function may refer to them via this Table.

Ilya

>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>
>>>>Richard.
>>>>
>>>>> Thanks,
>>>>> Ilya
>>>>> --
>>>>>
>>>>> gcc/
>>>>>
>>>>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>>
>>>>>         * tree-into-ssa.c: Include "target.h"
>>>>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>>         * tree-ssa-dom.c: Include "target.h"
>>>>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>>
>>>>>
>>>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>>>> index 981e9f4..8d48f6d 100644
>>>>> --- a/gcc/tree-into-ssa.c
>>>>> +++ b/gcc/tree-into-ssa.c
>>>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>>  #include "params.h"
>>>>>  #include "diagnostic-core.h"
>>>>>  #include "tree-into-ssa.h"
>>>>> +#include "target.h"
>>>>>
>>>>>
>>>>>  /* This file builds the SSA form for a function as described in:
>>>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>>>gimple_stmt_iterator gsi)
>>>>>      }
>>>>>
>>>>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>>>underlying
>>>>> -     symbol is marked for renaming.  */
>>>>> -  if (rewrite_uses_p (stmt))
>>>>> +     symbol is marked for renaming.
>>>>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>>>> +     renamed.  */
>>>>> +  if (rewrite_uses_p (stmt)
>>>>> +      && !(flag_check_pointer_bounds
>>>>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>>>>> +          && gimple_call_fndecl (stmt)
>>>>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>>>>      {
>>>>>        if (is_gimple_debug (stmt))
>>>>>         {
>>>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>>>> index 211bfcf..445278a 100644
>>>>> --- a/gcc/tree-ssa-dom.c
>>>>> +++ b/gcc/tree-ssa-dom.c
>>>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>>  #include "params.h"
>>>>>  #include "tree-ssa-threadedge.h"
>>>>>  #include "tree-ssa-dom.h"
>>>>> +#include "target.h"
>>>>>
>>>>>  /* This file implements optimizations on the dominator tree.  */
>>>>>
>>>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>>>    use_operand_p op_p;
>>>>>    ssa_op_iter iter;
>>>>>
>>>>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>>>>Checker
>>>>> +     should not be optimized.  Argument of the call is a default
>>>>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.
>>>>*/
>>>>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>>>GIMPLE_CALL)
>>>>> +    {
>>>>> +      tree fndecl = gimple_call_fndecl (stmt);
>>>>> +      if (fndecl == targetm.builtin_chkp_function
>>>>(BUILT_IN_CHKP_ARG_BND))
>>>>> +       return;
>>>>> +    }
>>>>> +
>>>>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>>>>      cprop_operand (stmt, op_p);
>>>>>  }
>>>
>>>

On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 5, 2013 at 1:02 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/4 Richard Biener <richard.guenther@gmail.com>:
>>>> Richard Biener <richard.guenther@gmail.com> wrote:
>>>>>On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich
>>>>><enkovich.gnu@gmail.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Here is a patch which hadles the problem with optimization of
>>>>>BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that
>>>>>argument of this call is a default SSA_NAME of the PARM_DECL whose
>>>>>bounds we want to get.  The problem is in optimizations which may
>>>>>replace arg with it's copy or a known value.  This patch suppress such
>>>>>modifications.
>>>>>
>>>>>This doesn't seem like a good fix.  I suppose you require the same on
>>>>>RTL, that is, have the incoming arg reg coalesced with the use?
>>>>>In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>>>
>>>> Btw, I would have chosen
>>>>
>>>> P_2 = __builtin_xyz (p_1, bound)
>>>> Call (p_2)
>>>>
>>>> Thus make the builtins a transparent pass-through which effectively binds parameter to their bound, removing the need for the artificial arguments and making propagations a non.issue
>>>>
>>>> Also, how does the feature interact with other extensions such as nested functions or optimizations like inlining?
>>>
>>> In RTL all incoming bounds are materialized into slot where bounds are
>>> passed and arg_bnd call is expanded into this slot.  Thus in RTL bound
>>> arg looks more like a regular arg.
>>
>> I don't care so much for RTL, but this description hints at that the
>> suggestion above would work (and it would eliminate all my concerns about
>> the representation on the GIMPLE level - you'd not even need this
>> strange POINTER_BOUNDS_TYPE as far as I can see.
>>
>>> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
>>> because it does not used in abnormal phi, shouldn't it?  Also it would
>>> prevent all optimizations for these SSA_NAMEs right?  Instrumentation
>>> is performed on the earlier stage, right after we build SSA. I think
>>> using abnormal phi flag and binding pointers with bounds via calls
>>> would prevent some useful optimizations.
>>
>> Well, what are the constraints that you need to avoid propagation in
>> the first place?
>
> For input parameter P I need to have
>   BOUNDS = __builtin_arg_bnd (P)
> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
> __builtin_arg_bnd (P) replacing P with its copy or some value. It
> makes call useless because removes information about parameter whose
> bounds we refer to. I want such optimization to ignore
> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
> there as arg.

How does a compilable testcase look like that shows how the default def
is used?  And what transforms break that use?  I really cannot see
how this would happen (and you didn't add a testcase).

Richard.

>>
>>> Many interprocedural optimizations require some support when work with
>>> instrumented calls.  Inlining support includes:
>>>   - replacement of arg_bnd calls with actual bounds passed to the inlined call
>>>   - replacement of retbnd call with bounds, returned by inlined function
>>>
>>> Not all IPA passes are fully enabled right now.  E.g. I restrict
>>> bounded value propagation in ipa-prop and bounded args in functions
>>> generated by ipa-split.  Such features will be enabled later.
>>>
>>> For nested functions I do not see much difference from checker point
>>> of view.  It just has an additional static chain param. Probably I
>>> miss here something.  I did just few tests with nested functions.
>>
>> I was thinking of
>>
>> int foo (char *s)
>> {
>>    int bar (void)
>>    {
>>       ... use bound of 's' of the containing function ...
>>       foo (q, and pass it along here for q)
>>    }
>> }
>>
>> that is references to the containing functions parameters and their
>
> All foo's locals referenced by nested bar here are placed into special
> structure and all bounds should be stored in appropriated entries of
> Bound Table.  Nested function may refer to them via this Table.
>
> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>
>>>>
>>>> Richard.
>>>>
>>>>
>>>>>Richard.
>>>>>
>>>>>> Thanks,
>>>>>> Ilya
>>>>>> --
>>>>>>
>>>>>> gcc/
>>>>>>
>>>>>> 2013-10-28  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>>>
>>>>>>         * tree-into-ssa.c: Include "target.h"
>>>>>>         (rewrite_update_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>>>         * tree-ssa-dom.c: Include "target.h"
>>>>>>         (cprop_into_stmt): Skip BUILT_IN_CHKP_ARG_BND calls.
>>>>>>
>>>>>>
>>>>>> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
>>>>>> index 981e9f4..8d48f6d 100644
>>>>>> --- a/gcc/tree-into-ssa.c
>>>>>> +++ b/gcc/tree-into-ssa.c
>>>>>> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>>>  #include "params.h"
>>>>>>  #include "diagnostic-core.h"
>>>>>>  #include "tree-into-ssa.h"
>>>>>> +#include "target.h"
>>>>>>
>>>>>>
>>>>>>  /* This file builds the SSA form for a function as described in:
>>>>>> @@ -1921,8 +1922,14 @@ rewrite_update_stmt (gimple stmt,
>>>>>gimple_stmt_iterator gsi)
>>>>>>      }
>>>>>>
>>>>>>    /* Rewrite USES included in OLD_SSA_NAMES and USES whose
>>>>>underlying
>>>>>> -     symbol is marked for renaming.  */
>>>>>> -  if (rewrite_uses_p (stmt))
>>>>>> +     symbol is marked for renaming.
>>>>>> +     Skip calls to BUILT_IN_CHKP_ARG_BND whose arg should never be
>>>>>> +     renamed.  */
>>>>>> +  if (rewrite_uses_p (stmt)
>>>>>> +      && !(flag_check_pointer_bounds
>>>>>> +          && (gimple_code (stmt) == GIMPLE_CALL)
>>>>>> +          && gimple_call_fndecl (stmt)
>>>>>> +          == targetm.builtin_chkp_function (BUILT_IN_CHKP_ARG_BND)))
>>>>>>      {
>>>>>>        if (is_gimple_debug (stmt))
>>>>>>         {
>>>>>> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
>>>>>> index 211bfcf..445278a 100644
>>>>>> --- a/gcc/tree-ssa-dom.c
>>>>>> +++ b/gcc/tree-ssa-dom.c
>>>>>> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>>>  #include "params.h"
>>>>>>  #include "tree-ssa-threadedge.h"
>>>>>>  #include "tree-ssa-dom.h"
>>>>>> +#include "target.h"
>>>>>>
>>>>>>  /* This file implements optimizations on the dominator tree.  */
>>>>>>
>>>>>> @@ -2266,6 +2267,16 @@ cprop_into_stmt (gimple stmt)
>>>>>>    use_operand_p op_p;
>>>>>>    ssa_op_iter iter;
>>>>>>
>>>>>> +  /* Call used to obtain bounds of input arg by Pointer Bounds
>>>>>Checker
>>>>>> +     should not be optimized.  Argument of the call is a default
>>>>>> +     SSA_NAME of PARM_DECL.  It should never be replaced by value.
>>>>>*/
>>>>>> +  if (flag_check_pointer_bounds && gimple_code (stmt) ==
>>>>>GIMPLE_CALL)
>>>>>> +    {
>>>>>> +      tree fndecl = gimple_call_fndecl (stmt);
>>>>>> +      if (fndecl == targetm.builtin_chkp_function
>>>>>(BUILT_IN_CHKP_ARG_BND))
>>>>>> +       return;
>>>>>> +    }
>>>>>> +
>>>>>>    FOR_EACH_SSA_USE_OPERAND (op_p, stmt, iter, SSA_OP_USE)
>>>>>>      cprop_operand (stmt, op_p);
>>>>>>  }
>>>>
>>>>

2013/11/5 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>
>> For input parameter P I need to have
>>   BOUNDS = __builtin_arg_bnd (P)
>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>> makes call useless because removes information about parameter whose
>> bounds we refer to. I want such optimization to ignore
>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>> there as arg.
>
> How does a compilable testcase look like that shows how the default def
> is used?  And what transforms break that use?  I really cannot see
> how this would happen (and you didn't add a testcase).

Here is a test source:

extern int bar1 (int *p);
extern int bar2 (int);

int foo (int *p)
{
  if (!p)
    return bar1 (p);
  return bar2 (10);
}

After instrumentation GIMPLE looks like:

foo (int * p)
{
  <unnamed type> __bound_tmp.0;
  int _1;
  int _6;
  int _8;

  <bb 6>:
  __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));

  <bb 2>:
  if (p_3(D) == 0B)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  _6 = bar1 (p_3(D), __bound_tmp.0_9);
  goto <bb 5>;

  <bb 4>:
  _8 = bar2 (10);

  <bb 5>:
  # _1 = PHI <_6(3), _8(4)>
  return _1;

}

Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):

foo (int * p)
{
  <unnamed type> __bound_tmp.0;
  int _1;
  int _6;
  int _8;

  <bb 2>:
  if (p_3(D) == 0B)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
  _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
  goto <bb 5>;

  <bb 4>:
  _8 = bar2 (10); [tail call]

  <bb 5>:
  # _1 = PHI <_6(3), _8(4)>
  return _1;

}

Now during expand or inline of foo I cannot determine the value for
__bound_tmp.0_9.

Ilya
>
> Richard.
>

On 11/04/13 06:27, Richard Biener wrote:
> On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> Hi,
>>
>> Here is a patch which hadles the problem with optimization of BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that argument of this call is a default SSA_NAME of the PARM_DECL whose bounds we want to get.  The problem is in optimizations which may replace arg with it's copy or a known value.  This patch suppress such modifications.
>
> This doesn't seem like a good fix.  I suppose you require the same on
> RTL, that is, have the incoming arg reg coalesced with the use?
> In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
I agree.  This seems like the wrong direction.

While I don't like abusing SSA_NAME_OCCURS_IN_ABNORMAL_PHI in this 
manner, setting that flag should give them precisely the behaviour they 
want.  That makes me think SSA_NAME_OCCURS_IN_ABNORMAL_PHI is poorly 
named.  But addressing that is separate from this patch.

Ilya, can you convert your code to set SSA_NAME_OCCURS_IN_ABNORMAL_PHI 
and drop this patch?


Jeff

On 11/05/13 05:02, Ilya Enkovich wrote:
>
> If I just set abnormal phi flag for SSA_NAME, SSA verifier should fail
> because it does not used in abnormal phi, shouldn't it?
Well, we can also change the verifier to not fail when it finds that 
flag set on a pointer when bounds checking is enabled.


   Also it would
> prevent all optimizations for these SSA_NAMEs right?  Instrumentation
> is performed on the earlier stage, right after we build SSA. I think
> using abnormal phi flag and binding pointers with bounds via calls
> would prevent some useful optimizations.
It certainly prevents some optimizations, but it's main purpose is to 
avoid certain copy propagation and coalescing.  If you don't set that 
flag, there's probably all kinds places you're going to have to hack up 
to keep the properties you desire for bounded pointers.

I would rather be a bit conservative and miss some optimizations by 
setting SSA_NAME_OCCURS_IN_ABNORMAL_PHI than be continually hacking up 
passes to special case bounded pointers.  We can obviously revisit this 
decision later.

Jeff

2013/11/5 Jeff Law <law@redhat.com>:
> On 11/04/13 06:27, Richard Biener wrote:
>>
>> On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich <enkovich.gnu@gmail.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> Here is a patch which hadles the problem with optimization of
>>> BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that argument
>>> of this call is a default SSA_NAME of the PARM_DECL whose bounds we want to
>>> get.  The problem is in optimizations which may replace arg with it's copy
>>> or a known value.  This patch suppress such modifications.
>>
>>
>> This doesn't seem like a good fix.  I suppose you require the same on
>> RTL, that is, have the incoming arg reg coalesced with the use?
>> In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>
> I agree.  This seems like the wrong direction.
>
> While I don't like abusing SSA_NAME_OCCURS_IN_ABNORMAL_PHI in this manner,
> setting that flag should give them precisely the behaviour they want.  That
> makes me think SSA_NAME_OCCURS_IN_ABNORMAL_PHI is poorly named.  But
> addressing that is separate from this patch.
>
> Ilya, can you convert your code to set SSA_NAME_OCCURS_IN_ABNORMAL_PHI and
> drop this patch?

OK. I'll try SSA_NAME_OCCURS_IN_ABNORMAL_PHI approach and see if it
causes any problems.

Thanks,
Ilya

>
>
> Jeff
>

On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>
>>> For input parameter P I need to have
>>>   BOUNDS = __builtin_arg_bnd (P)
>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>> makes call useless because removes information about parameter whose
>>> bounds we refer to. I want such optimization to ignore
>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>> there as arg.
>>
>> How does a compilable testcase look like that shows how the default def
>> is used?  And what transforms break that use?  I really cannot see
>> how this would happen (and you didn't add a testcase).
>
> Here is a test source:
>
> extern int bar1 (int *p);
> extern int bar2 (int);
>
> int foo (int *p)
> {
>   if (!p)
>     return bar1 (p);
>   return bar2 (10);
> }
>
> After instrumentation GIMPLE looks like:
>
> foo (int * p)
> {
>   <unnamed type> __bound_tmp.0;
>   int _1;
>   int _6;
>   int _8;
>
>   <bb 6>:
>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>
>   <bb 2>:
>   if (p_3(D) == 0B)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
>
>   <bb 3>:
>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>   goto <bb 5>;
>
>   <bb 4>:
>   _8 = bar2 (10);
>
>   <bb 5>:
>   # _1 = PHI <_6(3), _8(4)>
>   return _1;
>
> }
>
> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>
> foo (int * p)
> {
>   <unnamed type> __bound_tmp.0;
>   int _1;
>   int _6;
>   int _8;
>
>   <bb 2>:
>   if (p_3(D) == 0B)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
>
>   <bb 3>:
>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>   goto <bb 5>;
>
>   <bb 4>:
>   _8 = bar2 (10); [tail call]
>
>   <bb 5>:
>   # _1 = PHI <_6(3), _8(4)>
>   return _1;
>
> }
>
> Now during expand or inline of foo I cannot determine the value for
> __bound_tmp.0_9.

Well, but clearly you can see that bounds for p == NULL are
useless and thus there is no problem with the transform.

Also sth weird is going on as your arg_bnd call gets [return slot optimization].
It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.

Richard.

> Ilya
>>
>> Richard.
>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>
>>>> For input parameter P I need to have
>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>> makes call useless because removes information about parameter whose
>>>> bounds we refer to. I want such optimization to ignore
>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>> there as arg.
>>>
>>> How does a compilable testcase look like that shows how the default def
>>> is used?  And what transforms break that use?  I really cannot see
>>> how this would happen (and you didn't add a testcase).
>>
>> Here is a test source:
>>
>> extern int bar1 (int *p);
>> extern int bar2 (int);
>>
>> int foo (int *p)
>> {
>>   if (!p)
>>     return bar1 (p);
>>   return bar2 (10);
>> }
>>
>> After instrumentation GIMPLE looks like:
>>
>> foo (int * p)
>> {
>>   <unnamed type> __bound_tmp.0;
>>   int _1;
>>   int _6;
>>   int _8;
>>
>>   <bb 6>:
>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>
>>   <bb 2>:
>>   if (p_3(D) == 0B)
>>     goto <bb 3>;
>>   else
>>     goto <bb 4>;
>>
>>   <bb 3>:
>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>   goto <bb 5>;
>>
>>   <bb 4>:
>>   _8 = bar2 (10);
>>
>>   <bb 5>:
>>   # _1 = PHI <_6(3), _8(4)>
>>   return _1;
>>
>> }
>>
>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>
>> foo (int * p)
>> {
>>   <unnamed type> __bound_tmp.0;
>>   int _1;
>>   int _6;
>>   int _8;
>>
>>   <bb 2>:
>>   if (p_3(D) == 0B)
>>     goto <bb 3>;
>>   else
>>     goto <bb 4>;
>>
>>   <bb 3>:
>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>   goto <bb 5>;
>>
>>   <bb 4>:
>>   _8 = bar2 (10); [tail call]
>>
>>   <bb 5>:
>>   # _1 = PHI <_6(3), _8(4)>
>>   return _1;
>>
>> }
>>
>> Now during expand or inline of foo I cannot determine the value for
>> __bound_tmp.0_9.
>
> Well, but clearly you can see that bounds for p == NULL are
> useless and thus there is no problem with the transform.

This example just demonstrates the issue. I may use some var's address
in comparison with the similar result. And value does not always allow
to determine bounds, because pointers with the same value may have
different bounds.

>
> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.

Target is responsible for having return slot optimization here. And
target expands all such calls. So, do not see the problem here.

Ilya

>
> Richard.
>
>> Ilya
>>>
>>> Richard.
>>>

On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>
>>>>> For input parameter P I need to have
>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>> makes call useless because removes information about parameter whose
>>>>> bounds we refer to. I want such optimization to ignore
>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>> there as arg.
>>>>
>>>> How does a compilable testcase look like that shows how the default def
>>>> is used?  And what transforms break that use?  I really cannot see
>>>> how this would happen (and you didn't add a testcase).
>>>
>>> Here is a test source:
>>>
>>> extern int bar1 (int *p);
>>> extern int bar2 (int);
>>>
>>> int foo (int *p)
>>> {
>>>   if (!p)
>>>     return bar1 (p);
>>>   return bar2 (10);
>>> }
>>>
>>> After instrumentation GIMPLE looks like:
>>>
>>> foo (int * p)
>>> {
>>>   <unnamed type> __bound_tmp.0;
>>>   int _1;
>>>   int _6;
>>>   int _8;
>>>
>>>   <bb 6>:
>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>
>>>   <bb 2>:
>>>   if (p_3(D) == 0B)
>>>     goto <bb 3>;
>>>   else
>>>     goto <bb 4>;
>>>
>>>   <bb 3>:
>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>   goto <bb 5>;
>>>
>>>   <bb 4>:
>>>   _8 = bar2 (10);
>>>
>>>   <bb 5>:
>>>   # _1 = PHI <_6(3), _8(4)>
>>>   return _1;
>>>
>>> }
>>>
>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>
>>> foo (int * p)
>>> {
>>>   <unnamed type> __bound_tmp.0;
>>>   int _1;
>>>   int _6;
>>>   int _8;
>>>
>>>   <bb 2>:
>>>   if (p_3(D) == 0B)
>>>     goto <bb 3>;
>>>   else
>>>     goto <bb 4>;
>>>
>>>   <bb 3>:
>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>   goto <bb 5>;
>>>
>>>   <bb 4>:
>>>   _8 = bar2 (10); [tail call]
>>>
>>>   <bb 5>:
>>>   # _1 = PHI <_6(3), _8(4)>
>>>   return _1;
>>>
>>> }
>>>
>>> Now during expand or inline of foo I cannot determine the value for
>>> __bound_tmp.0_9.
>>
>> Well, but clearly you can see that bounds for p == NULL are
>> useless and thus there is no problem with the transform.
>
> This example just demonstrates the issue. I may use some var's address
> in comparison with the similar result. And value does not always allow
> to determine bounds, because pointers with the same value may have
> different bounds.

Some vars address can be used for better bounds deriving though.
Well, it clearly shows it's a "get me good results" issue and not
a correctness issue.  And for "good results" you trade in missed
general optimizations.  That doesn't sound very good to me.

Richard.

>>
>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>
> Target is responsible for having return slot optimization here. And
> target expands all such calls. So, do not see the problem here.
>
> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>>
>>>> Richard.
>>>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>
>>>>>> For input parameter P I need to have
>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>> makes call useless because removes information about parameter whose
>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>> there as arg.
>>>>>
>>>>> How does a compilable testcase look like that shows how the default def
>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>> how this would happen (and you didn't add a testcase).
>>>>
>>>> Here is a test source:
>>>>
>>>> extern int bar1 (int *p);
>>>> extern int bar2 (int);
>>>>
>>>> int foo (int *p)
>>>> {
>>>>   if (!p)
>>>>     return bar1 (p);
>>>>   return bar2 (10);
>>>> }
>>>>
>>>> After instrumentation GIMPLE looks like:
>>>>
>>>> foo (int * p)
>>>> {
>>>>   <unnamed type> __bound_tmp.0;
>>>>   int _1;
>>>>   int _6;
>>>>   int _8;
>>>>
>>>>   <bb 6>:
>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>
>>>>   <bb 2>:
>>>>   if (p_3(D) == 0B)
>>>>     goto <bb 3>;
>>>>   else
>>>>     goto <bb 4>;
>>>>
>>>>   <bb 3>:
>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>   goto <bb 5>;
>>>>
>>>>   <bb 4>:
>>>>   _8 = bar2 (10);
>>>>
>>>>   <bb 5>:
>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>   return _1;
>>>>
>>>> }
>>>>
>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>
>>>> foo (int * p)
>>>> {
>>>>   <unnamed type> __bound_tmp.0;
>>>>   int _1;
>>>>   int _6;
>>>>   int _8;
>>>>
>>>>   <bb 2>:
>>>>   if (p_3(D) == 0B)
>>>>     goto <bb 3>;
>>>>   else
>>>>     goto <bb 4>;
>>>>
>>>>   <bb 3>:
>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>   goto <bb 5>;
>>>>
>>>>   <bb 4>:
>>>>   _8 = bar2 (10); [tail call]
>>>>
>>>>   <bb 5>:
>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>   return _1;
>>>>
>>>> }
>>>>
>>>> Now during expand or inline of foo I cannot determine the value for
>>>> __bound_tmp.0_9.
>>>
>>> Well, but clearly you can see that bounds for p == NULL are
>>> useless and thus there is no problem with the transform.
>>
>> This example just demonstrates the issue. I may use some var's address
>> in comparison with the similar result. And value does not always allow
>> to determine bounds, because pointers with the same value may have
>> different bounds.
>
> Some vars address can be used for better bounds deriving though.
> Well, it clearly shows it's a "get me good results" issue and not
> a correctness issue.  And for "good results" you trade in missed
> general optimizations.  That doesn't sound very good to me.

That is definitely a correctness issue.  Compiler cannot assume bounds
by pointer value ignoring bounds passed to the function.
SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
 My original patch does not.

Ilya

>
> Richard.
>
>>>
>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>
>> Target is responsible for having return slot optimization here. And
>> target expands all such calls. So, do not see the problem here.
>>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>> Ilya
>>>>>
>>>>> Richard.
>>>>>

On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>
>>>>>>> For input parameter P I need to have
>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>> makes call useless because removes information about parameter whose
>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>> there as arg.
>>>>>>
>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>> how this would happen (and you didn't add a testcase).
>>>>>
>>>>> Here is a test source:
>>>>>
>>>>> extern int bar1 (int *p);
>>>>> extern int bar2 (int);
>>>>>
>>>>> int foo (int *p)
>>>>> {
>>>>>   if (!p)
>>>>>     return bar1 (p);
>>>>>   return bar2 (10);
>>>>> }
>>>>>
>>>>> After instrumentation GIMPLE looks like:
>>>>>
>>>>> foo (int * p)
>>>>> {
>>>>>   <unnamed type> __bound_tmp.0;
>>>>>   int _1;
>>>>>   int _6;
>>>>>   int _8;
>>>>>
>>>>>   <bb 6>:
>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>
>>>>>   <bb 2>:
>>>>>   if (p_3(D) == 0B)
>>>>>     goto <bb 3>;
>>>>>   else
>>>>>     goto <bb 4>;
>>>>>
>>>>>   <bb 3>:
>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>   goto <bb 5>;
>>>>>
>>>>>   <bb 4>:
>>>>>   _8 = bar2 (10);
>>>>>
>>>>>   <bb 5>:
>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>   return _1;
>>>>>
>>>>> }
>>>>>
>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>
>>>>> foo (int * p)
>>>>> {
>>>>>   <unnamed type> __bound_tmp.0;
>>>>>   int _1;
>>>>>   int _6;
>>>>>   int _8;
>>>>>
>>>>>   <bb 2>:
>>>>>   if (p_3(D) == 0B)
>>>>>     goto <bb 3>;
>>>>>   else
>>>>>     goto <bb 4>;
>>>>>
>>>>>   <bb 3>:
>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>   goto <bb 5>;
>>>>>
>>>>>   <bb 4>:
>>>>>   _8 = bar2 (10); [tail call]
>>>>>
>>>>>   <bb 5>:
>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>   return _1;
>>>>>
>>>>> }
>>>>>
>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>> __bound_tmp.0_9.
>>>>
>>>> Well, but clearly you can see that bounds for p == NULL are
>>>> useless and thus there is no problem with the transform.
>>>
>>> This example just demonstrates the issue. I may use some var's address
>>> in comparison with the similar result. And value does not always allow
>>> to determine bounds, because pointers with the same value may have
>>> different bounds.
>>
>> Some vars address can be used for better bounds deriving though.
>> Well, it clearly shows it's a "get me good results" issue and not
>> a correctness issue.  And for "good results" you trade in missed
>> general optimizations.  That doesn't sound very good to me.
>
> That is definitely a correctness issue.

Then please add a runtime testcase that fails before and passes after
your patch.

> Compiler cannot assume bounds
> by pointer value ignoring bounds passed to the function.

But it can certainly drop bounds to "unknown"?  You certainly cannot
rely on no such transform taking place.  Maybe this just shows that
your "refering to the parameter" is done wrong and shouldn't have
used the SSA name default def.  What do you do for parameters
that have their address taken btw?  You'll get

foo (void *p)
{
   p_2 = *p;
   __builtin_ia32_arg_bnd (p_2);

which isn't desired?

Hmm, unfortunately on x86_64 I get

./cc1 -quiet t.c -fcheck-pointer-bounds
t.c:1:0: error: -fcheck-pointers is not supported for this target
 extern int bar1 (int *p);
 ^

trying a simple testcase.  Grepping shows me no target supports
chkp_bound_mode ..?  What's this stuff in our tree that has no way
of testing it ... :/

Richard.

> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>  My original patch does not.
>
> Ilya
>
>>
>> Richard.
>>
>>>>
>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>
>>> Target is responsible for having return slot optimization here. And
>>> target expands all such calls. So, do not see the problem here.
>>>
>>> Ilya
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Ilya
>>>>>>
>>>>>> Richard.
>>>>>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>
>>>>>>>> For input parameter P I need to have
>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>> there as arg.
>>>>>>>
>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>
>>>>>> Here is a test source:
>>>>>>
>>>>>> extern int bar1 (int *p);
>>>>>> extern int bar2 (int);
>>>>>>
>>>>>> int foo (int *p)
>>>>>> {
>>>>>>   if (!p)
>>>>>>     return bar1 (p);
>>>>>>   return bar2 (10);
>>>>>> }
>>>>>>
>>>>>> After instrumentation GIMPLE looks like:
>>>>>>
>>>>>> foo (int * p)
>>>>>> {
>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>   int _1;
>>>>>>   int _6;
>>>>>>   int _8;
>>>>>>
>>>>>>   <bb 6>:
>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>
>>>>>>   <bb 2>:
>>>>>>   if (p_3(D) == 0B)
>>>>>>     goto <bb 3>;
>>>>>>   else
>>>>>>     goto <bb 4>;
>>>>>>
>>>>>>   <bb 3>:
>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>   goto <bb 5>;
>>>>>>
>>>>>>   <bb 4>:
>>>>>>   _8 = bar2 (10);
>>>>>>
>>>>>>   <bb 5>:
>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>   return _1;
>>>>>>
>>>>>> }
>>>>>>
>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>
>>>>>> foo (int * p)
>>>>>> {
>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>   int _1;
>>>>>>   int _6;
>>>>>>   int _8;
>>>>>>
>>>>>>   <bb 2>:
>>>>>>   if (p_3(D) == 0B)
>>>>>>     goto <bb 3>;
>>>>>>   else
>>>>>>     goto <bb 4>;
>>>>>>
>>>>>>   <bb 3>:
>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>   goto <bb 5>;
>>>>>>
>>>>>>   <bb 4>:
>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>
>>>>>>   <bb 5>:
>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>   return _1;
>>>>>>
>>>>>> }
>>>>>>
>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>> __bound_tmp.0_9.
>>>>>
>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>> useless and thus there is no problem with the transform.
>>>>
>>>> This example just demonstrates the issue. I may use some var's address
>>>> in comparison with the similar result. And value does not always allow
>>>> to determine bounds, because pointers with the same value may have
>>>> different bounds.
>>>
>>> Some vars address can be used for better bounds deriving though.
>>> Well, it clearly shows it's a "get me good results" issue and not
>>> a correctness issue.  And for "good results" you trade in missed
>>> general optimizations.  That doesn't sound very good to me.
>>
>> That is definitely a correctness issue.
>
> Then please add a runtime testcase that fails before and passes after
> your patch.
>
>> Compiler cannot assume bounds
>> by pointer value ignoring bounds passed to the function.
>
> But it can certainly drop bounds to "unknown"?  You certainly cannot
> rely on no such transform taking place.  Maybe this just shows that
> your "refering to the parameter" is done wrong and shouldn't have
> used the SSA name default def.  What do you do for parameters
> that have their address taken btw?  You'll get
>
> foo (void *p)
> {
>    p_2 = *p;
>    __builtin_ia32_arg_bnd (p_2);
>
> which isn't desired?

In this case you just make a load and bounds for p_2 are obtained
using bndldx call. I would be glad to use better way to refer to the
argument but I do not see one.

>
> Hmm, unfortunately on x86_64 I get
>
> ./cc1 -quiet t.c -fcheck-pointer-bounds
> t.c:1:0: error: -fcheck-pointers is not supported for this target
>  extern int bar1 (int *p);
>  ^
>
> trying a simple testcase.  Grepping shows me no target supports
> chkp_bound_mode ..?  What's this stuff in our tree that has no way
> of testing it ... :/

You should pass -mmpx to enable it on the target. Current version in
the mpx tree should build most of tests correctly but it is not
updated to the current state.

Ilya

>
> Richard.
>
>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>  My original patch does not.
>>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>>>
>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>
>>>> Target is responsible for having return slot optimization here. And
>>>> target expands all such calls. So, do not see the problem here.
>>>>
>>>> Ilya
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>> Ilya
>>>>>>>
>>>>>>> Richard.
>>>>>>>

On Wed, Nov 6, 2013 at 2:31 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> For input parameter P I need to have
>>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>>> there as arg.
>>>>>>>>
>>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>>
>>>>>>> Here is a test source:
>>>>>>>
>>>>>>> extern int bar1 (int *p);
>>>>>>> extern int bar2 (int);
>>>>>>>
>>>>>>> int foo (int *p)
>>>>>>> {
>>>>>>>   if (!p)
>>>>>>>     return bar1 (p);
>>>>>>>   return bar2 (10);
>>>>>>> }
>>>>>>>
>>>>>>> After instrumentation GIMPLE looks like:
>>>>>>>
>>>>>>> foo (int * p)
>>>>>>> {
>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>   int _1;
>>>>>>>   int _6;
>>>>>>>   int _8;
>>>>>>>
>>>>>>>   <bb 6>:
>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>>
>>>>>>>   <bb 2>:
>>>>>>>   if (p_3(D) == 0B)
>>>>>>>     goto <bb 3>;
>>>>>>>   else
>>>>>>>     goto <bb 4>;
>>>>>>>
>>>>>>>   <bb 3>:
>>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>>   goto <bb 5>;
>>>>>>>
>>>>>>>   <bb 4>:
>>>>>>>   _8 = bar2 (10);
>>>>>>>
>>>>>>>   <bb 5>:
>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>   return _1;
>>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>>
>>>>>>> foo (int * p)
>>>>>>> {
>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>   int _1;
>>>>>>>   int _6;
>>>>>>>   int _8;
>>>>>>>
>>>>>>>   <bb 2>:
>>>>>>>   if (p_3(D) == 0B)
>>>>>>>     goto <bb 3>;
>>>>>>>   else
>>>>>>>     goto <bb 4>;
>>>>>>>
>>>>>>>   <bb 3>:
>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>>   goto <bb 5>;
>>>>>>>
>>>>>>>   <bb 4>:
>>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>>
>>>>>>>   <bb 5>:
>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>   return _1;
>>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>>> __bound_tmp.0_9.
>>>>>>
>>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>>> useless and thus there is no problem with the transform.
>>>>>
>>>>> This example just demonstrates the issue. I may use some var's address
>>>>> in comparison with the similar result. And value does not always allow
>>>>> to determine bounds, because pointers with the same value may have
>>>>> different bounds.
>>>>
>>>> Some vars address can be used for better bounds deriving though.
>>>> Well, it clearly shows it's a "get me good results" issue and not
>>>> a correctness issue.  And for "good results" you trade in missed
>>>> general optimizations.  That doesn't sound very good to me.
>>>
>>> That is definitely a correctness issue.
>>
>> Then please add a runtime testcase that fails before and passes after
>> your patch.
>>
>>> Compiler cannot assume bounds
>>> by pointer value ignoring bounds passed to the function.
>>
>> But it can certainly drop bounds to "unknown"?  You certainly cannot
>> rely on no such transform taking place.  Maybe this just shows that
>> your "refering to the parameter" is done wrong and shouldn't have
>> used the SSA name default def.  What do you do for parameters
>> that have their address taken btw?  You'll get
>>
>> foo (void *p)
>> {
>>    p_2 = *p;
>>    __builtin_ia32_arg_bnd (p_2);
>>
>> which isn't desired?
>
> In this case you just make a load and bounds for p_2 are obtained
> using bndldx call. I would be glad to use better way to refer to the
> argument but I do not see one.

Err, my mistake - I meant

foo (void *p)
{
  p_2 = p;
  __builtin_ia32_arg_bnd (p_2);

>>
>> Hmm, unfortunately on x86_64 I get
>>
>> ./cc1 -quiet t.c -fcheck-pointer-bounds
>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>  extern int bar1 (int *p);
>>  ^
>>
>> trying a simple testcase.  Grepping shows me no target supports
>> chkp_bound_mode ..?  What's this stuff in our tree that has no way
>> of testing it ... :/
>
> You should pass -mmpx to enable it on the target. Current version in
> the mpx tree should build most of tests correctly but it is not
> updated to the current state.

> ./cc1 -quiet t.c -fcheck-pointer-bounds -mmpx
t.c:1:0: error: -fcheck-pointers is not supported for this target
 extern int bar1 (int *p);
 ^

hmm, maybe it's configure disabled somehow (this is just my dev tree).
But I still see no chkp_bound_mode implementation.

Note that POINTER_BOUNDS_TYPE shouldn't have been a new
tree code either but should be a RECORD_TYPE.  See how
va_list doesn't use a new TREE_CODE either.

Richard.

> Ilya
>
>>
>> Richard.
>>
>>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>>  My original patch does not.
>>>
>>> Ilya
>>>
>>>>
>>>> Richard.
>>>>
>>>>>>
>>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>>
>>>>> Target is responsible for having return slot optimization here. And
>>>>> target expands all such calls. So, do not see the problem here.
>>>>>
>>>>> Ilya
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Ilya
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 6, 2013 at 2:31 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> For input parameter P I need to have
>>>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>>>> there as arg.
>>>>>>>>>
>>>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>>>
>>>>>>>> Here is a test source:
>>>>>>>>
>>>>>>>> extern int bar1 (int *p);
>>>>>>>> extern int bar2 (int);
>>>>>>>>
>>>>>>>> int foo (int *p)
>>>>>>>> {
>>>>>>>>   if (!p)
>>>>>>>>     return bar1 (p);
>>>>>>>>   return bar2 (10);
>>>>>>>> }
>>>>>>>>
>>>>>>>> After instrumentation GIMPLE looks like:
>>>>>>>>
>>>>>>>> foo (int * p)
>>>>>>>> {
>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>   int _1;
>>>>>>>>   int _6;
>>>>>>>>   int _8;
>>>>>>>>
>>>>>>>>   <bb 6>:
>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>>>
>>>>>>>>   <bb 2>:
>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>     goto <bb 3>;
>>>>>>>>   else
>>>>>>>>     goto <bb 4>;
>>>>>>>>
>>>>>>>>   <bb 3>:
>>>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>>>   goto <bb 5>;
>>>>>>>>
>>>>>>>>   <bb 4>:
>>>>>>>>   _8 = bar2 (10);
>>>>>>>>
>>>>>>>>   <bb 5>:
>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>   return _1;
>>>>>>>>
>>>>>>>> }
>>>>>>>>
>>>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>>>
>>>>>>>> foo (int * p)
>>>>>>>> {
>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>   int _1;
>>>>>>>>   int _6;
>>>>>>>>   int _8;
>>>>>>>>
>>>>>>>>   <bb 2>:
>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>     goto <bb 3>;
>>>>>>>>   else
>>>>>>>>     goto <bb 4>;
>>>>>>>>
>>>>>>>>   <bb 3>:
>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>>>   goto <bb 5>;
>>>>>>>>
>>>>>>>>   <bb 4>:
>>>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>>>
>>>>>>>>   <bb 5>:
>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>   return _1;
>>>>>>>>
>>>>>>>> }
>>>>>>>>
>>>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>>>> __bound_tmp.0_9.
>>>>>>>
>>>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>>>> useless and thus there is no problem with the transform.
>>>>>>
>>>>>> This example just demonstrates the issue. I may use some var's address
>>>>>> in comparison with the similar result. And value does not always allow
>>>>>> to determine bounds, because pointers with the same value may have
>>>>>> different bounds.
>>>>>
>>>>> Some vars address can be used for better bounds deriving though.
>>>>> Well, it clearly shows it's a "get me good results" issue and not
>>>>> a correctness issue.  And for "good results" you trade in missed
>>>>> general optimizations.  That doesn't sound very good to me.
>>>>
>>>> That is definitely a correctness issue.
>>>
>>> Then please add a runtime testcase that fails before and passes after
>>> your patch.
>>>
>>>> Compiler cannot assume bounds
>>>> by pointer value ignoring bounds passed to the function.
>>>
>>> But it can certainly drop bounds to "unknown"?  You certainly cannot
>>> rely on no such transform taking place.  Maybe this just shows that
>>> your "refering to the parameter" is done wrong and shouldn't have
>>> used the SSA name default def.  What do you do for parameters
>>> that have their address taken btw?  You'll get
>>>
>>> foo (void *p)
>>> {
>>>    p_2 = *p;
>>>    __builtin_ia32_arg_bnd (p_2);
>>>
>>> which isn't desired?
>>
>> In this case you just make a load and bounds for p_2 are obtained
>> using bndldx call. I would be glad to use better way to refer to the
>> argument but I do not see one.
>
> Err, my mistake - I meant
>
> foo (void *p)
> {
>   p_2 = p;
>   __builtin_ia32_arg_bnd (p_2);

Well, it does not make much difference. Such params are allocated on
the stack by expand. So, here p_2 = p is load from memory and bounds
should be loaded by bndldx. Expand is responsible to store bounds
using bndstx when it allocates p on the stack.

>
>>>
>>> Hmm, unfortunately on x86_64 I get
>>>
>>> ./cc1 -quiet t.c -fcheck-pointer-bounds
>>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>>  extern int bar1 (int *p);
>>>  ^
>>>
>>> trying a simple testcase.  Grepping shows me no target supports
>>> chkp_bound_mode ..?  What's this stuff in our tree that has no way
>>> of testing it ... :/
>>
>> You should pass -mmpx to enable it on the target. Current version in
>> the mpx tree should build most of tests correctly but it is not
>> updated to the current state.
>
>> ./cc1 -quiet t.c -fcheck-pointer-bounds -mmpx
> t.c:1:0: error: -fcheck-pointers is not supported for this target
>  extern int bar1 (int *p);
>  ^
>
> hmm, maybe it's configure disabled somehow (this is just my dev tree).
> But I still see no chkp_bound_mode implementation.

I'll check it and probably update the branch.

>
> Note that POINTER_BOUNDS_TYPE shouldn't have been a new
> tree code either but should be a RECORD_TYPE.  See how
> va_list doesn't use a new TREE_CODE either.

va_list is either a pointer or pointer to a record (array of one
record) and is handled as a regular pointer to a regular record.
Bounds are not a record of two pointers at least due to binary format.
You do not pass it as regular record, use special bound mode for whole
record. Why to use RECORD_TYPE for bounds if it'll never be handled as
regular RECORD_TYPE?

Ilya

>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>>>  My original patch does not.
>>>>
>>>> Ilya
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>>>
>>>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>>>
>>>>>> Target is responsible for having return slot optimization here. And
>>>>>> target expands all such calls. So, do not see the problem here.
>>>>>>
>>>>>> Ilya
>>>>>>
>>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>>> Ilya
>>>>>>>>>
>>>>>>>>> Richard.
>>>>>>>>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 6, 2013 at 2:31 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> For input parameter P I need to have
>>>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>>>> there as arg.
>>>>>>>>>
>>>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>>>
>>>>>>>> Here is a test source:
>>>>>>>>
>>>>>>>> extern int bar1 (int *p);
>>>>>>>> extern int bar2 (int);
>>>>>>>>
>>>>>>>> int foo (int *p)
>>>>>>>> {
>>>>>>>>   if (!p)
>>>>>>>>     return bar1 (p);
>>>>>>>>   return bar2 (10);
>>>>>>>> }
>>>>>>>>
>>>>>>>> After instrumentation GIMPLE looks like:
>>>>>>>>
>>>>>>>> foo (int * p)
>>>>>>>> {
>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>   int _1;
>>>>>>>>   int _6;
>>>>>>>>   int _8;
>>>>>>>>
>>>>>>>>   <bb 6>:
>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>>>
>>>>>>>>   <bb 2>:
>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>     goto <bb 3>;
>>>>>>>>   else
>>>>>>>>     goto <bb 4>;
>>>>>>>>
>>>>>>>>   <bb 3>:
>>>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>>>   goto <bb 5>;
>>>>>>>>
>>>>>>>>   <bb 4>:
>>>>>>>>   _8 = bar2 (10);
>>>>>>>>
>>>>>>>>   <bb 5>:
>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>   return _1;
>>>>>>>>
>>>>>>>> }
>>>>>>>>
>>>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>>>
>>>>>>>> foo (int * p)
>>>>>>>> {
>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>   int _1;
>>>>>>>>   int _6;
>>>>>>>>   int _8;
>>>>>>>>
>>>>>>>>   <bb 2>:
>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>     goto <bb 3>;
>>>>>>>>   else
>>>>>>>>     goto <bb 4>;
>>>>>>>>
>>>>>>>>   <bb 3>:
>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>>>   goto <bb 5>;
>>>>>>>>
>>>>>>>>   <bb 4>:
>>>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>>>
>>>>>>>>   <bb 5>:
>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>   return _1;
>>>>>>>>
>>>>>>>> }
>>>>>>>>
>>>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>>>> __bound_tmp.0_9.
>>>>>>>
>>>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>>>> useless and thus there is no problem with the transform.
>>>>>>
>>>>>> This example just demonstrates the issue. I may use some var's address
>>>>>> in comparison with the similar result. And value does not always allow
>>>>>> to determine bounds, because pointers with the same value may have
>>>>>> different bounds.
>>>>>
>>>>> Some vars address can be used for better bounds deriving though.
>>>>> Well, it clearly shows it's a "get me good results" issue and not
>>>>> a correctness issue.  And for "good results" you trade in missed
>>>>> general optimizations.  That doesn't sound very good to me.
>>>>
>>>> That is definitely a correctness issue.
>>>
>>> Then please add a runtime testcase that fails before and passes after
>>> your patch.
>>>
>>>> Compiler cannot assume bounds
>>>> by pointer value ignoring bounds passed to the function.
>>>
>>> But it can certainly drop bounds to "unknown"?  You certainly cannot
>>> rely on no such transform taking place.  Maybe this just shows that
>>> your "refering to the parameter" is done wrong and shouldn't have
>>> used the SSA name default def.  What do you do for parameters
>>> that have their address taken btw?  You'll get
>>>
>>> foo (void *p)
>>> {
>>>    p_2 = *p;
>>>    __builtin_ia32_arg_bnd (p_2);
>>>
>>> which isn't desired?
>>
>> In this case you just make a load and bounds for p_2 are obtained
>> using bndldx call. I would be glad to use better way to refer to the
>> argument but I do not see one.
>
> Err, my mistake - I meant
>
> foo (void *p)
> {
>   p_2 = p;
>   __builtin_ia32_arg_bnd (p_2);
>
>>>
>>> Hmm, unfortunately on x86_64 I get
>>>
>>> ./cc1 -quiet t.c -fcheck-pointer-bounds
>>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>>  extern int bar1 (int *p);
>>>  ^
>>>
>>> trying a simple testcase.  Grepping shows me no target supports
>>> chkp_bound_mode ..?  What's this stuff in our tree that has no way
>>> of testing it ... :/
>>
>> You should pass -mmpx to enable it on the target. Current version in
>> the mpx tree should build most of tests correctly but it is not
>> updated to the current state.
>
>> ./cc1 -quiet t.c -fcheck-pointer-bounds -mmpx
> t.c:1:0: error: -fcheck-pointers is not supported for this target
>  extern int bar1 (int *p);
>  ^
>
> hmm, maybe it's configure disabled somehow (this is just my dev tree).
> But I still see no chkp_bound_mode implementation.

Sorry, I missed you use your dev tree. Only first 7 patches of ~30
have been committed so far and therefore no support yet. Currently
full implementation is available in mpx branch only.

Ilya

>
> Note that POINTER_BOUNDS_TYPE shouldn't have been a new
> tree code either but should be a RECORD_TYPE.  See how
> va_list doesn't use a new TREE_CODE either.
>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>>>  My original patch does not.
>>>>
>>>> Ilya
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>>>
>>>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>>>
>>>>>> Target is responsible for having return slot optimization here. And
>>>>>> target expands all such calls. So, do not see the problem here.
>>>>>>
>>>>>> Ilya
>>>>>>
>>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>>> Ilya
>>>>>>>>>
>>>>>>>>> Richard.
>>>>>>>>>

On Wed, Nov 6, 2013 at 3:04 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>> On Wed, Nov 6, 2013 at 2:31 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> For input parameter P I need to have
>>>>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>>>>> there as arg.
>>>>>>>>>>
>>>>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>>>>
>>>>>>>>> Here is a test source:
>>>>>>>>>
>>>>>>>>> extern int bar1 (int *p);
>>>>>>>>> extern int bar2 (int);
>>>>>>>>>
>>>>>>>>> int foo (int *p)
>>>>>>>>> {
>>>>>>>>>   if (!p)
>>>>>>>>>     return bar1 (p);
>>>>>>>>>   return bar2 (10);
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> After instrumentation GIMPLE looks like:
>>>>>>>>>
>>>>>>>>> foo (int * p)
>>>>>>>>> {
>>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>>   int _1;
>>>>>>>>>   int _6;
>>>>>>>>>   int _8;
>>>>>>>>>
>>>>>>>>>   <bb 6>:
>>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>>>>
>>>>>>>>>   <bb 2>:
>>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>>     goto <bb 3>;
>>>>>>>>>   else
>>>>>>>>>     goto <bb 4>;
>>>>>>>>>
>>>>>>>>>   <bb 3>:
>>>>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>>>>   goto <bb 5>;
>>>>>>>>>
>>>>>>>>>   <bb 4>:
>>>>>>>>>   _8 = bar2 (10);
>>>>>>>>>
>>>>>>>>>   <bb 5>:
>>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>>   return _1;
>>>>>>>>>
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>>>>
>>>>>>>>> foo (int * p)
>>>>>>>>> {
>>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>>   int _1;
>>>>>>>>>   int _6;
>>>>>>>>>   int _8;
>>>>>>>>>
>>>>>>>>>   <bb 2>:
>>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>>     goto <bb 3>;
>>>>>>>>>   else
>>>>>>>>>     goto <bb 4>;
>>>>>>>>>
>>>>>>>>>   <bb 3>:
>>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>>>>   goto <bb 5>;
>>>>>>>>>
>>>>>>>>>   <bb 4>:
>>>>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>>>>
>>>>>>>>>   <bb 5>:
>>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>>   return _1;
>>>>>>>>>
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>>>>> __bound_tmp.0_9.
>>>>>>>>
>>>>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>>>>> useless and thus there is no problem with the transform.
>>>>>>>
>>>>>>> This example just demonstrates the issue. I may use some var's address
>>>>>>> in comparison with the similar result. And value does not always allow
>>>>>>> to determine bounds, because pointers with the same value may have
>>>>>>> different bounds.
>>>>>>
>>>>>> Some vars address can be used for better bounds deriving though.
>>>>>> Well, it clearly shows it's a "get me good results" issue and not
>>>>>> a correctness issue.  And for "good results" you trade in missed
>>>>>> general optimizations.  That doesn't sound very good to me.
>>>>>
>>>>> That is definitely a correctness issue.
>>>>
>>>> Then please add a runtime testcase that fails before and passes after
>>>> your patch.
>>>>
>>>>> Compiler cannot assume bounds
>>>>> by pointer value ignoring bounds passed to the function.
>>>>
>>>> But it can certainly drop bounds to "unknown"?  You certainly cannot
>>>> rely on no such transform taking place.  Maybe this just shows that
>>>> your "refering to the parameter" is done wrong and shouldn't have
>>>> used the SSA name default def.  What do you do for parameters
>>>> that have their address taken btw?  You'll get
>>>>
>>>> foo (void *p)
>>>> {
>>>>    p_2 = *p;
>>>>    __builtin_ia32_arg_bnd (p_2);
>>>>
>>>> which isn't desired?
>>>
>>> In this case you just make a load and bounds for p_2 are obtained
>>> using bndldx call. I would be glad to use better way to refer to the
>>> argument but I do not see one.
>>
>> Err, my mistake - I meant
>>
>> foo (void *p)
>> {
>>   p_2 = p;
>>   __builtin_ia32_arg_bnd (p_2);
>
> Well, it does not make much difference. Such params are allocated on
> the stack by expand. So, here p_2 = p is load from memory and bounds
> should be loaded by bndldx. Expand is responsible to store bounds
> using bndstx when it allocates p on the stack.

The parameter is incoming in a register still.

>>
>>>>
>>>> Hmm, unfortunately on x86_64 I get
>>>>
>>>> ./cc1 -quiet t.c -fcheck-pointer-bounds
>>>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>>>  extern int bar1 (int *p);
>>>>  ^
>>>>
>>>> trying a simple testcase.  Grepping shows me no target supports
>>>> chkp_bound_mode ..?  What's this stuff in our tree that has no way
>>>> of testing it ... :/
>>>
>>> You should pass -mmpx to enable it on the target. Current version in
>>> the mpx tree should build most of tests correctly but it is not
>>> updated to the current state.
>>
>>> ./cc1 -quiet t.c -fcheck-pointer-bounds -mmpx
>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>  extern int bar1 (int *p);
>>  ^
>>
>> hmm, maybe it's configure disabled somehow (this is just my dev tree).
>> But I still see no chkp_bound_mode implementation.
>
> I'll check it and probably update the branch.

I'm looking at trunk which has loads of this stuff already.

>>
>> Note that POINTER_BOUNDS_TYPE shouldn't have been a new
>> tree code either but should be a RECORD_TYPE.  See how
>> va_list doesn't use a new TREE_CODE either.
>
> va_list is either a pointer or pointer to a record (array of one
> record) and is handled as a regular pointer to a regular record.
> Bounds are not a record of two pointers at least due to binary format.
> You do not pass it as regular record, use special bound mode for whole
> record. Why to use RECORD_TYPE for bounds if it'll never be handled as
> regular RECORD_TYPE?

Then you only can update it completely?  Seems awfully target specific
for sth introduced at the GIMPLE level.  Oh well.

Richard.

> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>
>>>>
>>>> Richard.
>>>>
>>>>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>>>>  My original patch does not.
>>>>>
>>>>> Ilya
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>>>
>>>>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>>>>
>>>>>>> Target is responsible for having return slot optimization here. And
>>>>>>> target expands all such calls. So, do not see the problem here.
>>>>>>>
>>>>>>> Ilya
>>>>>>>
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>>> Ilya
>>>>>>>>>>
>>>>>>>>>> Richard.
>>>>>>>>>>

2013/11/6 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 6, 2013 at 3:04 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Nov 6, 2013 at 2:31 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>> 2013/11/6 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> For input parameter P I need to have
>>>>>>>>>>>>   BOUNDS = __builtin_arg_bnd (P)
>>>>>>>>>>>> to somehow refer to bounds of P in GIMPLE.  Optimizations may modify
>>>>>>>>>>>> __builtin_arg_bnd (P) replacing P with its copy or some value. It
>>>>>>>>>>>> makes call useless because removes information about parameter whose
>>>>>>>>>>>> bounds we refer to. I want such optimization to ignore
>>>>>>>>>>>> __builtin_arg_bnd calls and always leave default SSA_NAME of PARM_DECL
>>>>>>>>>>>> there as arg.
>>>>>>>>>>>
>>>>>>>>>>> How does a compilable testcase look like that shows how the default def
>>>>>>>>>>> is used?  And what transforms break that use?  I really cannot see
>>>>>>>>>>> how this would happen (and you didn't add a testcase).
>>>>>>>>>>
>>>>>>>>>> Here is a test source:
>>>>>>>>>>
>>>>>>>>>> extern int bar1 (int *p);
>>>>>>>>>> extern int bar2 (int);
>>>>>>>>>>
>>>>>>>>>> int foo (int *p)
>>>>>>>>>> {
>>>>>>>>>>   if (!p)
>>>>>>>>>>     return bar1 (p);
>>>>>>>>>>   return bar2 (10);
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> After instrumentation GIMPLE looks like:
>>>>>>>>>>
>>>>>>>>>> foo (int * p)
>>>>>>>>>> {
>>>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>>>   int _1;
>>>>>>>>>>   int _6;
>>>>>>>>>>   int _8;
>>>>>>>>>>
>>>>>>>>>>   <bb 6>:
>>>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (p_3(D));
>>>>>>>>>>
>>>>>>>>>>   <bb 2>:
>>>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>>>     goto <bb 3>;
>>>>>>>>>>   else
>>>>>>>>>>     goto <bb 4>;
>>>>>>>>>>
>>>>>>>>>>   <bb 3>:
>>>>>>>>>>   _6 = bar1 (p_3(D), __bound_tmp.0_9);
>>>>>>>>>>   goto <bb 5>;
>>>>>>>>>>
>>>>>>>>>>   <bb 4>:
>>>>>>>>>>   _8 = bar2 (10);
>>>>>>>>>>
>>>>>>>>>>   <bb 5>:
>>>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>>>   return _1;
>>>>>>>>>>
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> Here is optimized GIMPLE (if I do not apply my changes in tree-ssa-dom.c):
>>>>>>>>>>
>>>>>>>>>> foo (int * p)
>>>>>>>>>> {
>>>>>>>>>>   <unnamed type> __bound_tmp.0;
>>>>>>>>>>   int _1;
>>>>>>>>>>   int _6;
>>>>>>>>>>   int _8;
>>>>>>>>>>
>>>>>>>>>>   <bb 2>:
>>>>>>>>>>   if (p_3(D) == 0B)
>>>>>>>>>>     goto <bb 3>;
>>>>>>>>>>   else
>>>>>>>>>>     goto <bb 4>;
>>>>>>>>>>
>>>>>>>>>>   <bb 3>:
>>>>>>>>>>   __bound_tmp.0_9 = __builtin_ia32_arg_bnd (0B); [return slot optimization]
>>>>>>>>>>   _6 = bar1 (0B, __bound_tmp.0_9); [tail call]
>>>>>>>>>>   goto <bb 5>;
>>>>>>>>>>
>>>>>>>>>>   <bb 4>:
>>>>>>>>>>   _8 = bar2 (10); [tail call]
>>>>>>>>>>
>>>>>>>>>>   <bb 5>:
>>>>>>>>>>   # _1 = PHI <_6(3), _8(4)>
>>>>>>>>>>   return _1;
>>>>>>>>>>
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> Now during expand or inline of foo I cannot determine the value for
>>>>>>>>>> __bound_tmp.0_9.
>>>>>>>>>
>>>>>>>>> Well, but clearly you can see that bounds for p == NULL are
>>>>>>>>> useless and thus there is no problem with the transform.
>>>>>>>>
>>>>>>>> This example just demonstrates the issue. I may use some var's address
>>>>>>>> in comparison with the similar result. And value does not always allow
>>>>>>>> to determine bounds, because pointers with the same value may have
>>>>>>>> different bounds.
>>>>>>>
>>>>>>> Some vars address can be used for better bounds deriving though.
>>>>>>> Well, it clearly shows it's a "get me good results" issue and not
>>>>>>> a correctness issue.  And for "good results" you trade in missed
>>>>>>> general optimizations.  That doesn't sound very good to me.
>>>>>>
>>>>>> That is definitely a correctness issue.
>>>>>
>>>>> Then please add a runtime testcase that fails before and passes after
>>>>> your patch.
>>>>>
>>>>>> Compiler cannot assume bounds
>>>>>> by pointer value ignoring bounds passed to the function.
>>>>>
>>>>> But it can certainly drop bounds to "unknown"?  You certainly cannot
>>>>> rely on no such transform taking place.  Maybe this just shows that
>>>>> your "refering to the parameter" is done wrong and shouldn't have
>>>>> used the SSA name default def.  What do you do for parameters
>>>>> that have their address taken btw?  You'll get
>>>>>
>>>>> foo (void *p)
>>>>> {
>>>>>    p_2 = *p;
>>>>>    __builtin_ia32_arg_bnd (p_2);
>>>>>
>>>>> which isn't desired?
>>>>
>>>> In this case you just make a load and bounds for p_2 are obtained
>>>> using bndldx call. I would be glad to use better way to refer to the
>>>> argument but I do not see one.
>>>
>>> Err, my mistake - I meant
>>>
>>> foo (void *p)
>>> {
>>>   p_2 = p;
>>>   __builtin_ia32_arg_bnd (p_2);
>>
>> Well, it does not make much difference. Such params are allocated on
>> the stack by expand. So, here p_2 = p is load from memory and bounds
>> should be loaded by bndldx. Expand is responsible to store bounds
>> using bndstx when it allocates p on the stack.
>
> The parameter is incoming in a register still.

It does not matter. In GIMPLE it is still load. During expand
assign_parms allocates slot on the stack for it and stores reg to it.
DECL_RTL for PARM_DECL is set to stack slot.

I just added code to store corresponding bounds for stored param (I
did not send this patch yet).

Ilya
>
>>>
>>>>>
>>>>> Hmm, unfortunately on x86_64 I get
>>>>>
>>>>> ./cc1 -quiet t.c -fcheck-pointer-bounds
>>>>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>>>>  extern int bar1 (int *p);
>>>>>  ^
>>>>>
>>>>> trying a simple testcase.  Grepping shows me no target supports
>>>>> chkp_bound_mode ..?  What's this stuff in our tree that has no way
>>>>> of testing it ... :/
>>>>
>>>> You should pass -mmpx to enable it on the target. Current version in
>>>> the mpx tree should build most of tests correctly but it is not
>>>> updated to the current state.
>>>
>>>> ./cc1 -quiet t.c -fcheck-pointer-bounds -mmpx
>>> t.c:1:0: error: -fcheck-pointers is not supported for this target
>>>  extern int bar1 (int *p);
>>>  ^
>>>
>>> hmm, maybe it's configure disabled somehow (this is just my dev tree).
>>> But I still see no chkp_bound_mode implementation.
>>
>> I'll check it and probably update the branch.
>
> I'm looking at trunk which has loads of this stuff already.
>
>>>
>>> Note that POINTER_BOUNDS_TYPE shouldn't have been a new
>>> tree code either but should be a RECORD_TYPE.  See how
>>> va_list doesn't use a new TREE_CODE either.
>>
>> va_list is either a pointer or pointer to a record (array of one
>> record) and is handled as a regular pointer to a regular record.
>> Bounds are not a record of two pointers at least due to binary format.
>> You do not pass it as regular record, use special bound mode for whole
>> record. Why to use RECORD_TYPE for bounds if it'll never be handled as
>> regular RECORD_TYPE?
>
> Then you only can update it completely?  Seems awfully target specific
> for sth introduced at the GIMPLE level.  Oh well.
>
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>> Ilya
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>> SSA_NAME_OCCURS_IN_ABNORMAL_PHI usage results in missed optimizations.
>>>>>>  My original patch does not.
>>>>>>
>>>>>> Ilya
>>>>>>
>>>>>>>
>>>>>>> Richard.
>>>>>>>
>>>>>>>>>
>>>>>>>>> Also sth weird is going on as your arg_bnd call gets [return slot optimization].
>>>>>>>>> It seems that your POINTER_BOUNDS_TYPE is seen as aggregate somehow.
>>>>>>>>
>>>>>>>> Target is responsible for having return slot optimization here. And
>>>>>>>> target expands all such calls. So, do not see the problem here.
>>>>>>>>
>>>>>>>> Ilya
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Richard.
>>>>>>>>>
>>>>>>>>>> Ilya
>>>>>>>>>>>
>>>>>>>>>>> Richard.
>>>>>>>>>>>

Hi,

On Wed, 6 Nov 2013, Ilya Enkovich wrote:

> >>> Well, but clearly you can see that bounds for p == NULL are
> >>> useless and thus there is no problem with the transform.
> >>
> >> This example just demonstrates the issue. I may use some var's address
> >> in comparison with the similar result. And value does not always allow
> >> to determine bounds, because pointers with the same value may have
> >> different bounds.
> >
> > Some vars address can be used for better bounds deriving though.
> > Well, it clearly shows it's a "get me good results" issue and not
> > a correctness issue.  And for "good results" you trade in missed
> > general optimizations.  That doesn't sound very good to me.
> 
> That is definitely a correctness issue.  Compiler cannot assume bounds
> by pointer value ignoring bounds passed to the function.

Of course they can.  They assume "unbounded" for unknown bounds and emit 
correct code.  It's merely more conservative than you'd like (as the 
function got passed tighter bounds than unbounded), and hence no 
correctness but a QoI problem.


Ciao,
Michael.

2013/11/6 Michael Matz <matz@suse.de>:
> Hi,
>
> On Wed, 6 Nov 2013, Ilya Enkovich wrote:
>
>> >>> Well, but clearly you can see that bounds for p == NULL are
>> >>> useless and thus there is no problem with the transform.
>> >>
>> >> This example just demonstrates the issue. I may use some var's address
>> >> in comparison with the similar result. And value does not always allow
>> >> to determine bounds, because pointers with the same value may have
>> >> different bounds.
>> >
>> > Some vars address can be used for better bounds deriving though.
>> > Well, it clearly shows it's a "get me good results" issue and not
>> > a correctness issue.  And for "good results" you trade in missed
>> > general optimizations.  That doesn't sound very good to me.
>>
>> That is definitely a correctness issue.  Compiler cannot assume bounds
>> by pointer value ignoring bounds passed to the function.
>
> Of course they can.  They assume "unbounded" for unknown bounds and emit
> correct code.  It's merely more conservative than you'd like (as the
> function got passed tighter bounds than unbounded), and hence no
> correctness but a QoI problem.

Well, it depends on what we call 'correct' behavior for checker. I
suppose that bound violation caught on -O0 and not caught on -O2 is a
compiler issue. That is why I call it correctness issue. When you
instrument your code you intentionally slow your program down to
improve security. Compiler should have strong reason to behave
contrary. Of course, there should be some trade off. But in this
particular case I do not see why we should make pointer unchecked. If
I just drop this patch and use undefined bounds in all cases arg_bnd
call is optimized, I wont get any performance gain.

Ilya

>
>
> Ciao,
> Michael.

2013/11/6 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2013/11/5 Jeff Law <law@redhat.com>:
>> On 11/04/13 06:27, Richard Biener wrote:
>>>
>>> On Thu, Oct 31, 2013 at 10:02 AM, Ilya Enkovich <enkovich.gnu@gmail.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Here is a patch which hadles the problem with optimization of
>>>> BUILT_IN_CHKP_ARG_BND calls.  Pointer Bounds Checker expects that argument
>>>> of this call is a default SSA_NAME of the PARM_DECL whose bounds we want to
>>>> get.  The problem is in optimizations which may replace arg with it's copy
>>>> or a known value.  This patch suppress such modifications.
>>>
>>>
>>> This doesn't seem like a good fix.  I suppose you require the same on
>>> RTL, that is, have the incoming arg reg coalesced with the use?
>>> In that case better set SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>>
>> I agree.  This seems like the wrong direction.
>>
>> While I don't like abusing SSA_NAME_OCCURS_IN_ABNORMAL_PHI in this manner,
>> setting that flag should give them precisely the behaviour they want.  That
>> makes me think SSA_NAME_OCCURS_IN_ABNORMAL_PHI is poorly named.  But
>> addressing that is separate from this patch.
>>
>> Ilya, can you convert your code to set SSA_NAME_OCCURS_IN_ABNORMAL_PHI and
>> drop this patch?
>
> OK. I'll try SSA_NAME_OCCURS_IN_ABNORMAL_PHI approach and see if it
> causes any problems.

Seems flag works fine.  All my tests pass.  No need to make additional
changes in verifiers etc.  Fix will be included into instrumentation
pass patch.  This patch is dropped.

Thanks,
Ilya

>
> Thanks,
> Ilya
>
>>
>>
>> Jeff
>>