2013/11/20 Richard Biener <richard.guent...@gmail.com>: > On Wed, Nov 20, 2013 at 11:55 AM, Ilya Enkovich <enkovich....@gmail.com> > wrote: >> 2013/11/20 Richard Biener <richard.guent...@gmail.com>: >>> On Tue, Nov 19, 2013 at 9:18 PM, Ilya Enkovich <enkovich....@gmail.com> >>> wrote: >>>> 2013/11/19 Jeff Law <l...@redhat.com>: >>>>> On 11/19/13 05:20, Ilya Enkovich wrote: >>>>>> >>>>>> 2013/11/19 Richard Biener <richard.guent...@gmail.com>: >>>>>>> >>>>>>> On Mon, Nov 18, 2013 at 8:12 PM, Ilya Enkovich <enkovich....@gmail.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> 2013/11/18 Jeff Law <l...@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 >>>>>