This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


2013/11/20 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Nov 20, 2013 at 11:55 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 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).
>
> Sure, but you want to have this feature for a production compiler as
> for sure you are going to miss an odd sequence of transforms that
> breaks your assumptions.

There still should be a way to detect such cases.  If all unsupported
cases are silently ignored then user never knows about it and never
reports the problem.

I may fix expand changes to make it less vulnerable to different code
modifications and get no ICEs without fixes in optimizers (well,
except fix in tree-ssa-ccp.c where instrumentation breaks assumption
for data flow between stack save/restore builtins). But even with
stable expand I would still want to have all those changes in
optimizers to get better checker quality.

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]