2013/11/6 Richard Biener <richard.guent...@gmail.com>:
> 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?
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.
>>>>>>>
