On Wed, Nov 6, 2013 at 12:59 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> 2013/11/6 Richard Biener <richard.guent...@gmail.com>:
>> On Wed, Nov 6, 2013 at 12:00 PM, Ilya Enkovich <enkovich....@gmail.com>
>> wrote:
>>> 2013/11/6 Richard Biener <richard.guent...@gmail.com>:
>>>> On Tue, Nov 5, 2013 at 3:24 PM, Ilya Enkovich <enkovich....@gmail.com>
>>>> wrote:
>>>>> 2013/11/5 Richard Biener <richard.guent...@gmail.com>:
>>>>>> On Tue, Nov 5, 2013 at 1:52 PM, Ilya Enkovich <enkovich....@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.
>>>>>>
