This is the mail archive of the 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, Pointer Bounds Checker 14/x] Passes [12/n] Optimize string functions

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

 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
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

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.


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