On Wed, Jun 11, 2014 at 12:44:29PM +0400, Ilya Enkovich wrote:
> 2014-06-11 12:22 GMT+04:00 Jakub Jelinek <ja...@redhat.com>:
> > On Wed, Jun 11, 2014 at 12:14:14PM +0400, Ilya Enkovich wrote:
> >> This patch adds instrumented code support for strlen optimization.
> >>
> >> Bootstrapped and tested on linux-x86_64.
> >>
> >> Does it look OK?
> >
> > Ugh, this looks terribly ugly. So you are changing the meaning of arguments
> > of every single builtin that has pointer arguments, so all spots that
> > work with those builtins have to take into account
> > gimple_call_with_bounds_p and do something different?
> > That is very error prone and maintainance nightmare.
>
> All such builtins are had to be handled anyway. It is possible to use
> new codes for instrumented builtins, but this would not change the
> handling code much. It is also possible to use something like
> gimple_call_nobnd_arg to use same arg numbers for both instrumented
> and not instrumented calls. This would still require additional
> support when we replace calls.
That just brings the question why is all the instrumentation introduced so
early that it affects practically all GIMPLE passes?
I mean, mudflap, also a bounded pointer implementation, could get away with
two passes and practically no changes to the rest of the compiler, asan
also has two passes and almost no changes to the rest, why does this need
to affect everything? If that is because you need to clone functions,
which you indeed need to do at IPA time, perhaps you can just clone the
functions but not add the instrumentation to the whole IL at that point,
just note you've created a bounded pointer clone, and then just shortly
before expansion in a new pass add the instrumentation, plus hook into
expansion that it is expanded properly?
Changes all over the compiler for a rarely used feature most GCC developers
will not be able to even test is going to cause substantial maintainance
issues.
Jakub
