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 [14/25] Function splitting
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Jeff Law <law at redhat dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>
- Date: Thu, 21 Nov 2013 13:31:27 +0400
- Subject: Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Authentication-results: sourceware.org; auth=none
- References: <20131118102208 dot GH21297 at msticlxl57 dot ims dot intel dot com> <528A57AD dot 3070909 at redhat dot com> <CAMbmDYZxnwV=n9TkHuWTaj_ZPD0LBUTYwzEyCwd7ZS-MRdm0Fg at mail dot gmail dot com> <528A5A60 dot 1030206 at redhat dot com> <CAMbmDYa2ASRiVFryrk9YOGsdPROxKHuj1b=M0X6GWuAm3VEnEA at mail dot gmail dot com> <528A5EB4 dot 4050000 at redhat dot com> <CAMbmDYYGPzcfS5mJOUNddZ63C66Kt33s-ZOL81128E-tNUrcag at mail dot gmail dot com> <CAFiYyc1afDA2TjP0o4Vry2diKw8NryWLNw3vpAzOsK5gAwnuVQ at mail dot gmail dot com> <CAMbmDYa-1r1cofVF4L+ZEFCc-+CCM-BJMRFFgYT-tbmObhcfiA at mail dot gmail dot com> <528BB898 dot 2060902 at redhat dot com> <CAMbmDYbow-SLK3t42NX4EnmbVpvLq=jG4pr3p67ZD-1VCOJSCQ at mail dot gmail dot com> <CAFiYyc38SegEBEUsisKHcywYDx2hb8t06Z9nT=L1t9aec20xvA at mail dot gmail dot com> <CAFiYyc0jda5z=rtu8+hByxURoa9dm0n7cMqRDJUwOosN5sfcMg at mail dot gmail dot com>
2013/11/20 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 20, 2013 at 10:57 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Tue, Nov 19, 2013 at 9:18 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/19 Jeff Law <law@redhat.com>:
>>>> On 11/19/13 05:20, Ilya Enkovich wrote:
>>>>>
>>>>> 2013/11/19 Richard Biener <richard.guenther@gmail.com>:
>>>>>>
>>>>>> On Mon, Nov 18, 2013 at 8:12 PM, Ilya Enkovich <enkovich.gnu@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> 2013/11/18 Jeff Law <law@redhat.com>:
>>>>>>>>
>>>>>>>> On 11/18/13 11:27, Ilya Enkovich wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> How does pointer passed to regular function differ from pointer passed
>>>>>>>>> to splitted function? How do I know then which pointer is to be passed
>>>>>>>>> with bounds and wchich one is not? Moreover current ABI does not allow
>>>>>>>>> to pass bounds with no pointer or pass bounds for some pointers in the
>>>>>>>>> call only.
>>>>>>>>
>>>>>>>>
>>>>>>>> But I don't see any case in function splitting where we're going to
>>>>>>>> want to
>>>>>>>> pass the pointer without the bounds. If you want the former, you're
>>>>>>>> going
>>>>>>>> to want the latter.
>>>>>>>
>>>>>>>
>>>>>>> There are at least cases when checks are eliminated or when lots of
>>>>>>> pointer usages are accompanied with few checks performed earlier (e.g.
>>>>>>> we are working with array). In such cases splitted part may easily get
>>>>>>> no bounds.
>>>>>>>
>>>>>>>>
>>>>>>>> I really don't see why you need to do anything special here. At the
>>>>>>>> most an
>>>>>>>> assert in the splitting code to ensure that you don't have a situation
>>>>>>>> where
>>>>>>>> there's mixed pointers with bounds and pointers without bounds should
>>>>>>>> be all
>>>>>>>> you need or that you passed a bounds with no associated pointer :-)
>>>>>>>
>>>>>>>
>>>>>>> It would also require generation of proper bind_bounds calls in the
>>>>>>> original function and arg_bounds calls in a separated part. So,
>>>>>>> special support is required.
>>>>>>
>>>>>>
>>>>>> Well, only to keep proper instrumentation. I hope code still works
>>>>>> (doesn't trap) when optimizations "wreck" the bounds? Thus all
>>>>>> these patches are improving bounds propagation but are not required
>>>>>> for correctness? If so please postpone all of them until after the
>>>>>> initial support is merged. If not, please make sure BND instrumentation
>>>>>> works conservatively when optimizations wreck it.
>>>>>
>>>>>
>>>>> All patches I sent for optimization passes are required to avoid ICEs
>>>>> when compiling instrumented code.
>>>>
>>>> Then I think we're going to need to understand them in more detail. That's
>>>> going to mean testcases, probably dumps and some commentary about what went
>>>> wrong.
>>>>
>>>> I can't speak for Richi, but when optimizations get disabled, I tend to want
>>>> to really understand why and make sure we're not papering over a larger
>>>> problem.
>>>>
>>>> The tail recursion elimination one we're discussing now is a great example.
>>>> At this point I understand the problem you're running into, but I'm still
>>>> trying to wrap my head around the implications of the funny semantics of
>>>> __builtin_arg_bounds and how they may cause other problems.
>>>
>>> Root of all problems if implicit data flow hidden in arg_bounds and
>>> bind_bounds. Calls consume bounds and compiler does not know it. And
>>> input bounds are always expressed via arg_bounds calls and never
>>> expressed via formal args. Obviously optimizers have to be taught
>>> about these data dependencies to work correctly.
>>>
>>> I agree semantics of arg_bounds call creates many issues for
>>> optimizers but currently I do not see a better replacement for it.
>>
>> But it looks incredibly fragile if you ICE once something you don't like
>> happens. You should be able to easily detect the case and "punt",
>> that is, drop to non-instrumented aka invalidating bounds.
>>
>> Thus, I really really don't like these patches. They hint at some
>> deeper problem with the overall design (or the HW feature or the
>> accompaning ABI).
>
> Note that this, the intrusiveness of the feature and the questionable
> gain makes me question whether GCC should have support for this
> feature (and whether we really should rush this in this late).
>
> Thus, I hereby formally ask to push back this feature to 4.10.
I think you overestimate the intrusiveness of the checker. Necessity
of changes in optimization passes is artificial and is used to get
maximum checking quality. It can be easily made immune for different
code transformation by simple changes in the process of
instrumentation expand (I have a fix for that already). With that
change only pass itself, support for bound args during expand, support
in i386 target and minor infrastructure changes are required (e.g.
flag in varpool_node, bounds_constants). Changes in inline,
propagation, SRA, tail recursion, strlen, function splitting, string
function builtins expand would become optional and affect checking
quality only.
Also note that all changes do not affect compilation process when no
instrumentation is used.
Please reconsider your decision about pushing it to 4.10 taking that
into account.
Thanks,
Ilya
>
> Thanks,
> Richard.
>
>> Richard.
>>
>>> Ilya
>>>
>>>>
>>>>
>>>> jeff
>>>>
- References:
- [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting
- Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting