<div>Hello,</div><div>Let me state the steps taken, so that we can try to schedule partial patch </div><div>sending for review:</div><div><br></div><div>1 adding missing trivial empty destructors to Value-derived classes for avoiding of</div>
<div>calls to compiler-generated destructors.</div><div>2 implementing deleter2 which will call specific destructor based on Value-type</div><div>could be using mock delete ptr implementation in first release to make build not break,</div>
<div>to be replaced later with real implementation.</div><div>3 replacing calls to delete Value* with calls to deleter2.</div><div>4 adding special iplist2 as future replacement of iplist<Value-derived> to enforce </div>
<div>element destruction by deleter2 instead of operator delete.</div><div>5 replacing iplist<Value-derived> uses with iplist2.</div><div>6 adding realThis to User to retrieve the real allocated pointer from User* (which is kind </div>
<div>of fake due to tweaked placement new used for allocating User). </div><div>7 changing visibility of destructors to protected to cause the compiler to expose the </div><div>now-banned destructor calls (responsibility of which will be taken by deleter2 method).</div>
<div>8 making deleter2 friend of Value-derived classes to make detructors visible to deleter2</div><div>9 finally due to ld problem which is used with gcc compiler for building llvm </div><div>on some platforms, PseudoSourceValue had to be moved to lib/VMCore from </div>
<div>lib/CodeGen.</div><div>10 in lib/CodeGen/pseudosourcevalue.cpp, PSVGlobalsTy had to be modified to </div><div>create and destroy Values dynamically.</div><div><br></div><div>Please note that 3,4,5 make up some 60-70% of this patch as well as its hard to separate </div>
<div>out clearly 4 from 5.</div><div><br></div><div>Please state which steps would You see included in separate partial patches to</div><div>make work easier to be scheduled and managed from Your side. (on a side note review tool </div>
<div>like used in google chrome would be great replacement of this process, sigh).</div><div><br></div><div>Also if possible please refer me to more specific workitems by Doug, so that </div><div>I can try to reproduce his way of solving similar issue.</div>
<div><br></div><div>Best regards,</div><div>Pawel Kunio</div><div><br></div><div><br><div class="gmail_quote">On Fri, Feb 18, 2011 at 1:15 AM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div><div></div><div class="h5"><br>
On Feb 16, 2011, at 9:13 PM, pawel kunio wrote:<br>
<br>
> Hello,<br>
> In a meantime of preparing 2 tests for PR9067, I came with my 2nd and hopefully now more<br>
> successful attempt at now-infamous PR889 devirtualize Value destructor.<br>
><br>
> Please find patches for llvm and clang attached to this mail. As I can see from mingw build<br>
> environment, it will need moving PseudoSourceValue.cpp from lib/CodeGen to lib/VMCore<br>
> after applying patch (seems not to be required on vc build env, as order of libraries given<br>
> to linker is not meaningful there).<br>
><br>
> Please note that destructors have been moved into protected part of interface as well as<br>
> empty destructors were implemented for all Value-derived classes to help in catching all<br>
> destructions.<br>
<br>
</div></div>Hi Pawel,<br>
<br>
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.<br>

<font color="#888888"><br>
-Chris<br>
<br>
</font></blockquote></div><br></div>