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

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

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.
>>>>>