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

[Richard Biener <richard dot guenther at gmail dot 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.

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
>>