<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;">Chris wrote:</span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;"><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier"></font></div></blockquote><br><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">On Mar 26, 2008, at 3:16 PM, Gabor Greif wrote:</font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> here comes the patch for the first wave of Use class size reduction.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> I have split it into 3 files, corresponding to</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> - header changes</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> - implementation changes</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> - applications</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">nice!</font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> This at the moment does not contain the description how the</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> size of the Use class will be reduced from 16 to 12 bytes,</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> I am going to send that in a separate patch.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">Right, sounds great.</font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> This wave primarily consists of changes that are needed</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> to allocate objects with a variable number of embedded <Use>s.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">Ok.</font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> Where the number of <Use>s is constant in the lifetime of an object,</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> I preferred to keep the 'new Instr(...)' syntax. Otherwise I have</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> introduced static 'Create(...)' methods, which are used instead of the</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> 'new Instr(...)' construct.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">The bad thing about this is that it is inconsistent. I'd prefer to </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">have consistent use of ::Create for all IR classes to make it easier </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">to learn the API. That said, a temporary moment of inconsistency is </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">ok: you could commit this part, then switch the remaining classes over.</font></div></blockquote><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div>Yeah, this is a wart, but it is only temporary as you say.</span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;">I already have some sed scripts that aid the transition :-)</span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;"><br></span></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 13px;"><br><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> These replace the constructors and the</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> constructors become private/protected. The arguments of the 'Create'</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> methods are identical to the corresponding constructors.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> Essentially at the moment all introduced 'operator new's end up </i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> calling</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> '::operator new(size_t)', so there should be no functionality change </i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> at all.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">><i> This will change in subsequent waves.</i></font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">This basically looks good, some thoughts for the future:</font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">1) Please (eventually) don't make the 'operator new' override be </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">public. I'd actually prefer all memory alloc/dealloc stuff to be done </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">privately to the (vmcore) .cpp files, not exposed through the header.</font></div></blockquote><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Okay. I guess these relpacement 'operator new' will go away in favor of</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Foo::Create methods.</div><br><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">2) Eventually we'll need to make the dtor private as well.</font></div></blockquote><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Okay. So no more 'delete xxx;". But then we must be careful to</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">never call the Foo::Destroy() method on a NULL ptr.</div><br><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">3) Make sure that make check and some reasonable subset of llvm-test </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">passes with this patch :)</font></div></blockquote><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">I have never run llvm-test in the past. Is it just checking it out and</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">following a readme?</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><blockquote type="cite" class=""><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Courier; min-height: 16px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">Finally, please update llvm-gcc 4.2 as well when you commit this. </font></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><font face="Courier" size="4" style="font: 13.0px Courier">I'll update clang after you commit.</font></div></blockquote><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Well, I have patches for both (attached). But I am caught between a rock and a</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">hard place, because Xcode 2.4.1-built llvm-gcc ICEs-out on the</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Release llvm (which is needed as per README-LLVM). So I have a hard</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">time to run those tests. I keep trying. Possibly softlinking the</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Release dirs to the Debug ones.</div><span></span></span></font></div></body></html>