On 06/26/14 12:46, Jan Hubicka wrote:
So you've added this at -O2, what is the general compile-time
impact? Would it make more sense to instead have it be part of -O3,
particularly since ICF is rarely going to improve performance (sans
icache issues).

I think code size optimization not sacrifying any (or too much of) performance 
are
generally very welcome at -O2.  Compared to LLVM and Microsoft compilers we are
on code bloat side at -O2.
I'm not so much worried about runtime performance here, but compile-time performance. ICF would seem like a general win as we're likely going to be more icache efficient.

So, just to be clear, if the compile-time impact is minimal, then I'll fully support -O2, but my worry is that it won't be minimal :(


Is returning TRUE really the conservatively correct thing to do in
the absence of aliasing information?  Isn't that case really "I
don't know" in which case the proper return value is FALSE?

I think with -fno-strict-aliasing the set should be 0 (Richi?) and thus we can
compare for equality.  We probably can be on agressive side and let 0 alias
set prevail the non-0.  But that can be done incrementally.
I'd think it should be zero in the -fno-strict-aliasing case. My concern was that in the -fno-strict-aliasing case we seems to always assume the objects are the same. Is that the safe thing to do?

That hints that the comment for the function needs tweaking. It really doesn't say anything about what the return values really mean.


There are few, like we can ignore "weak" or "visibility" attribute because we do
produce alias with proper visibility anyway.  My plan is to start removing those
attributes from declarations once they are turned into suitable representation
in symbol table (or for attributes like const/noreturn/pure where we have 
explicit
decl flags).  This will make our life bit easier later, too.

We probably then can whitelist some attributes, but I would deal with this 
later.
Sure, I don't mind going with the conservative approach and iterating to remove some of the limitations.


Yep, there are no resonable orders on it.  If function starts same in source 
code they ought
to end up same here.  Plan was to first match for exact equality and then play 
with
smarter tricks here.
Yea, that's fine too. It's conservatively correct and we may find that there just isn't much to gain by doing hte dfs walk to build indices and such for the CFG. I guess ultimately I just want someone to look at that issue and evaluate if what we're doing now is "good enough" or if we're missing most of the benefit because of something like bb index stability or something dumb like that.


Yep, the pass has grown up to be rather long. The gimple equality testing is the
main body of work, so perhaps doing this in separate file is good idea.
I'd certainly like to see that happen both because of its size and because I think those bits are useful in other contexts.

Overall, most of the stuff looks quite reasonable.

jeff

Reply via email to