On Thu, Nov 21, 2013 at 10:31 AM, Ilya Enkovich <enkovich....@gmail.com> wrote: > 2013/11/20 Richard Biener <richard.guent...@gmail.com>: >> On Wed, Nov 20, 2013 at 10:57 AM, Richard Biener >> <richard.guent...@gmail.com> wrote: >>> 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. >>> >>> 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.
The point is that we are still pondering over the design and stage1 is basically over. Richard. > Thanks, > Ilya > >> >> Thanks, >> Richard. >> >>> Richard. >>> >>>> Ilya >>>> >>>>> >>>>> >>>>> jeff >>>>>
