Re: [PATCH, MPX, 2/X] Pointers Checker [14/25] Function splitting

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

2013/11/20 Richard Biener <richard.guenther@gmail.com>:
> 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.

Actually, it is easy detect such cases and invalidate bounds each time
some optimization bounds data flow.  With such feature 5-6 of sent
patches would be unnecessary to successfully build instrumented code
on -O2, but without them quality of checks would be much lower (mostly
due to inline).

Ilya

>
> 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).
>
> Richard.
>
>> Ilya
>>
>>>
>>>
>>> jeff
>>>