Re: [PATCH, Pointer Bounds Checker 14/x] Passes [12/n] Optimize string functions

[Jeff Law <law at redhat dot com>]

On 10/10/14 08:19, Ilya Enkovich wrote:
> > So is the purpose here to expose the checks that would normally be
> > done in the mem* routines to their caller in the hopes that doing
> > so will expose redundant checks?  Or is there some other reason?
>
> There are few reasons to replace instrumented string functions:
>
> 1.  As you said following redundant checks elimination may remove
> checks for some string functions
> 2.  By default functions like memcpy
> should assume pointers are copied and copy bounds.  If we know
> pointers are not copied, we may use faster version with no bounds
> copy
> 3.  If we avoid both checks and bounds copy then it is a
> candidate for existing string function calls inlining in expand pass
Perfect.  So this belongs in a comment in the code.

> I thought tests will be added later.
Did you already post them?  There's been so many patches I'm starting to 
lose track :-)

For future reference, when you break a submission down into logical 
hunks, including the tests in those logical hunks helps.  I realize the 
MPX work isn't as well suited for that kind of breakdown, but it's worth 
keeping in mind.


 I have ~250 tests to commit.
> Will check I have tests for optimizations.
Excellent.


 BTW this particular
> optimization cann't work until we have instrumented builtin calls.
Yea, hopefully we'll get to that before close of stage1.

> > It's a nit, but I'd tend to write that as:
> >
> > if (!fndecl_nochk) continue;
> >
> > fndecl = fndecl_nochk gimple_call_set_fndecl (stmt, fndecl);
>
> There is one more assignment to fndecl above which makes your version
> nonequivalent.
I had assumed the gimple_call_set_fndecl was a nop if we didn't change 
the fndecl.  Is that not the case?

> > I'm a bit surprised we don't have this kind of capability already
> > broken out.  But assuming that's the case, can you go ahead and
> > break that out into its own little helper function?    You don't
> > need to find all the cases where we're doing this kind of thing
> > today, just create the helper function and use it in your new
> > code.
>
> I could miss such function  (looked in cfg hooks and tree-cfg.h).
> Hopefully someone will correct me if it is so.
Thanks.  I suspect everyone has just done their own implementation 
inline like you did.  It's something I'll be keeping my eye out for in 
others' code so we can funnel everyone into your new function.  ISTM 
many speculative optimizations are going to need that kind of helper.
> Taking into account not instrumented builtin calls I suppose this
> patch goes into a next builtin related series.  But here is a version
> with changes.
Yea, I think you're right.  I think this is OK when the builtins are done.

jeff