[llvm-commits] my 2nd attempt at now-infamous PR889

[pawel kunio]

Hello,
Let me state the steps taken, so that we can try to schedule partial patch
sending for review:

1 adding missing trivial empty destructors to Value-derived classes for
avoiding of
calls to compiler-generated destructors.
2 implementing deleter2 which will call specific destructor based on
Value-type
could be using mock delete ptr implementation in first release to make build
not break,
to be replaced later with real implementation.
3 replacing calls to delete Value* with calls to deleter2.
4 adding special iplist2 as future replacement of iplist<Value-derived> to
enforce
element destruction by deleter2 instead of operator delete.
5 replacing iplist<Value-derived> uses with iplist2.
6 adding realThis to User to retrieve the real allocated pointer from User*
(which is kind
of fake due to tweaked placement new used for allocating User).
7 changing visibility of destructors to protected to cause the compiler to
expose the
now-banned destructor calls (responsibility of which will be taken by
deleter2 method).
8 making deleter2 friend of Value-derived classes to make detructors visible
to deleter2
9 finally due to ld problem which is used with gcc compiler for building
llvm
on some platforms, PseudoSourceValue had to be moved to lib/VMCore from
lib/CodeGen.
10 in lib/CodeGen/pseudosourcevalue.cpp, PSVGlobalsTy had to be modified to
create and destroy Values dynamically.

Please note that 3,4,5 make up some 60-70% of this patch as well as its hard
to separate
out clearly 4 from 5.

Please state which steps would You see included in separate partial patches
to
make work easier to be scheduled and managed from Your side. (on a side note
review tool
like used in google chrome would be great replacement of this process,
sigh).

Also if possible please refer me to more specific workitems by Doug, so
that
I can try to reproduce his way of solving similar issue.

Best regards,
Pawel Kunio


On Fri, Feb 18, 2011 at 1:15 AM, Chris Lattner <clattner at apple.com> wrote:

>
> On Feb 16, 2011, at 9:13 PM, pawel kunio wrote:
>
> > Hello,
> > In a meantime of preparing 2 tests for PR9067, I came with my 2nd and
> hopefully now more
> > successful attempt at now-infamous PR889 devirtualize Value destructor.
> >
> > Please find patches for llvm and clang attached to this mail. As I can
> see from mingw build
> > environment, it will need moving PseudoSourceValue.cpp from lib/CodeGen
> to lib/VMCore
> > after applying patch (seems not to be required on vc build env, as order
> of libraries given
> > to linker is not meaningful there).
> >
> > Please note that destructors have been moved into protected part of
> interface as well as
> > empty destructors were implemented for all Value-derived classes to help
> in catching all
> > destructions.
>
> Hi Pawel,
>
> Thank you for working on this!  Unfortunately, this patch is way too
> monolithic to review like this though, could you split it out into a series
> of obvious changes?  Doug recently "devirtualized" the clang decl hierarchy
> by devirtualizing one method at a time, which worked really well.
>
> -Chris
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110218/d8111b84/attachment.html>