On 12/02/2015 06:38 PM, Dmitry Vyukov wrote:
One thing to consider would
be whether you really need this split between O0/optimize versions, or
whether you can find a place in the queue where to insert it
unconditionally. Have you considered this at all or did you just follow
asan/tsan?
I inserted the pass just before asan/tsan because it looks like the
right place for it. If we do it after asan, it will insert coverage
for all asan-emited BBs which is highly undesirable. I also think it
is a good idea to run a bunch of optimizations before coverage pass to
not emit too many coverage callbacks (but I can't say that I am very
knowledgeable in this area). FWIW clang does the same: coverage passes
run just before asan/tsan.
There's one other thing I want to put out there. Is this kind of thing
maybe what plugins were invented for? I don't really like the concept of
plugins, but it seems to me that this sort of thing might be an
application for them.
+public:
+ static pass_data pd ()
+ {
+ static const pass_data data =
I think a static data member would be better than the unnecessary pd ()
function. This is also unlike existing practice, and I wonder how others
think about it. IMO a fairly strong case could be made that if we're using
C++, then this sort of thing ought to be part of the class definition.
I vary name of the pass depending on the O0 template argument (again
following asan):
O0 ? "sancov_O0" : "sancov", /* name */
If we call it "sancov" always, then I can make it just a global var
(as all other passes in gcc).
Or I can make it a static variable of the template class and move
definition of the class (as you proposed).
What would you prefer?
I think I prefer the static var of the template class. I just wonder why
we don't have the pass_data for all the existing passes as static data
members? I'm sure there's some reason.
asan also distinguishes the name between asan/asan0. I'd either follow
that naming convention, or remove the _O0 variant for all three of them.
I lean towards the latter.
Bernd
