On Wed, Nov 20, 2013 at 11:41 AM, Ilya Enkovich <enkovich....@gmail.com> wrote: > 2013/11/20 Richard Biener <richard.guent...@gmail.com>: >> On Tue, Nov 19, 2013 at 8:02 PM, Jeff Law <l...@redhat.com> wrote: >>> On 11/18/13 14:03, Ilya Enkovich wrote: >>>> >>>> 2013/11/19 Jeff Law <l...@redhat.com>: >>>>> >>>>> On 11/18/13 12:16, Ilya Enkovich wrote: >>>>>> >>>>>> >>>>>> With current recursion elimination we will have: >>>>>> >>>>>> test (int *param1) >>>>>> { >>>>>> <bb1>: >>>>>> >>>>>> <bb2>: >>>>>> _7 = PHI<param1(D) (bb1), _6 (bb2)> >>>>>> bounds2 = __builtin_arg_bounds (_7) -- WRONG >>>>> >>>>> >>>>> I wouldn't say it's wrong. It's conservatively correct if properly >>>>> implemented. Why precisely do you consider this wrong? If your code >>>>> can't >>>>> handle it, it really should be fixed. >>>> >>>> >>>> It is wrong because __builtin_arg_bounds is used to get bounds of >>>> input parameter and PHI node here is not an input parameter. >>> >>> OK, now we're getting somewhere. So we've got this odd little function >>> which only works on parameters. I can't help but feel this is a bit of >>> mis-design coming back to haunt us and I wonder if we're going to see other >>> instances of this kind of problem. >> >> Right. >> >>> There's something just wrong with the semantics of __builtin_arg_bounds, but >>> I'm having trouble putting my finger on it. >> >> Well, the issue is that it accepts any pointer argument as input. >> I originally thought it may just get an integer constant argument - the >> argument position. It really seems to me that we have >> __builtin_arg_boundsN (), but to avoid having N builtins we specify N >> via an argument to the function. But as it may not change we should >> use something that cannot change - like a constant. A constant >> that identifies a parameter is one that gives its position for example. > > I have tried to pass constant for arg_bounds. It still requires > additional modification of optimization passes (including tail > recursion, param propagation, inline etc.) But having constant as an > argument you really cannot detect errors. E.g. if you inline and do > not fix inlined arg_bounds calls correctly, you may not get ICE but > get wrong program which uses wrong bounds and produces false bounds > violations.
Yeah, that's why I refrained from suggesting this earlier ;) You lose the connection between "bounds for argument X" and "function entry value for argument X". Richard. >> >> Note that any restrictions you impose on what is "valid" for GIMPLE >> should be verified in tree-cfg.c:verify_gimple_* - in the >> __builtin_arg_bounds call case in verify_gimple_call. > > Will add it. > > Thanks, > Ilya > >> >> Richard. >> >>> >>> Correctl >>>> >>>> handling of __builtin_arg_bounds in this 'wrong' example requires >>>> adding it a PHI node semantics based on it's arg. For me it seems >>>> more complex then generating a regular PHI node for bounds and makes >>>> GIMPLE less readable. >>> >>> But presumably you have code to merge bound information already, right? You >>> need that for PHI nodes. Can't you use that to walk up the use-def chains >>> and build the bounds information? >>> >>> Or if you want to fix the tailcall stuff, you can start down that direction. >>> I don't think that'll fix the nagging concerns about the overall semantics >>> of builtin_arg_bounds, but it might be enough for you to go forward. >>> >>> jeff