This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 6 Nov 2013 18:12:49 +0400
- Subject: Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Authentication-results: sourceware.org; auth=none
- References: <20131031090213 dot GC54327 at msticlxl57 dot ims dot intel dot com> <CAFiYyc3gBc=CHDranqXo49NG6yFMSc3d-N8buFtjSS5G5Nga5w at mail dot gmail dot com> <4115420e-30be-445c-a68a-46168b939c01 at email dot android dot com> <CAMbmDYaQ+2YKDsUXVfUxw3VJ=njiZm6uOTduwi8tS9rNoEoJ2g at mail dot gmail dot com> <CAFiYyc26Rks2N3G8MaeUKeq-sCmih1nNW_rrCfbiLN+CCTCv5A at mail dot gmail dot com> <CAMbmDYZiCKCqV2k4ch4JnTsMy4bTnAwEWJw79Z=213PLyGmfew at mail dot gmail dot com> <CAFiYyc2Vc-PSv40TQ9Wwrkjw=QWpvd-Xta6hbJ6K_8_jo4mtPA at mail dot gmail dot com> <CAMbmDYbzBPwzRvzeiQ+iMV9oFRwLbdeQU1LiftK-fAWYQtEdpg at mail dot gmail dot com> <CAFiYyc1G_QOj539NoSCd_H6RHfLGozypvNtnKvWt6vwAMfvohQ at mail dot gmail dot com> <CAMbmDYZa9jwnuZVLFrG+RjW+F5Z4iu6CtQ+E6Zycjq6JncMW0A at mail dot gmail dot com> <CAFiYyc17wHyHT7GhYs9khZ2kGhqi2ed6mtrUwYmRCfFXqNe8Cg at mail dot gmail dot com> <CAMbmDYbiv3pZEo3OjPgLwVQ2GzomifofO91EHZPsz9pEFKmcQQ at mail dot gmail dot com> <CAFiYyc0Q-efoL6onifQT5ygGbP69eMoLUqmdK7tPer0mUeHw9g at mail dot gmail dot com> <CAMbmDYZndDsojoo6qoDSV2rfxg+8+HMGX_mk95ekx4cgiVd4RA at mail dot gmail dot com> <CAFiYyc0EX2sgbUre6Dp95NzdaccFUHg6W1iJTh+_YA2ETyDSzg at mail dot gmail dot com>
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.
>>>>>>>>>
- References:
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.
- Re: [PATCH, MPX, 2/X] Pointers Checker [7/25] Suppress BUILT_IN_CHKP_ARG_BND optimizations.