Chris wrote:> On Mar 26, 2008, at 3:16 PM, Gabor Greif wrote: > > here comes the patch for the first wave of Use class size reduction. > > > > I have split it into 3 files, corresponding to > > - header changes > > - implementation changes > > - applications > > nice! > > > This at the moment does not contain the description how the > > size of the Use class will be reduced from 16 to 12 bytes, > > I am going to send that in a separate patch. > > Right, sounds great. > > > This wave primarily consists of changes that are needed > > to allocate objects with a variable number of embedded <Use>s. > > Ok. > > > Where the number of <Use>s is constant in the lifetime of an object, > > I preferred to keep the 'new Instr(...)' syntax. Otherwise I have > > introduced static 'Create(...)' methods, which are used instead > of the > > 'new Instr(...)' construct. > > The bad thing about this is that it is inconsistent. I'd prefer to > have consistent use of ::Create for all IR classes to make it easier > to learn the API. That said, a temporary moment of inconsistency is > ok: you could commit this part, then switch the remaining classes > over.Yeah, this is a wart, but it is only temporary as you say. I already have some sed scripts that aid the transition :-)> > > These replace the constructors and the > > constructors become private/protected. The arguments of the 'Create' > > methods are identical to the corresponding constructors. > > Essentially at the moment all introduced 'operator new's end up > > calling > > '::operator new(size_t)', so there should be no functionality change > > at all. > > This will change in subsequent waves. > > > This basically looks good, some thoughts for the future: > > 1) Please (eventually) don't make the 'operator new' override be > public. I'd actually prefer all memory alloc/dealloc stuff to be done > privately to the (vmcore) .cpp files, not exposed through the header.Okay. I guess these relpacement 'operator new' will go away in favor of Foo::Create methods.> > 2) Eventually we'll need to make the dtor private as well.Okay. So no more 'delete xxx;". But then we must be careful to never call the Foo::Destroy() method on a NULL ptr.> > 3) Make sure that make check and some reasonable subset of llvm-test > passes with this patch :)I have never run llvm-test in the past. Is it just checking it out and following a readme?> > Finally, please update llvm-gcc 4.2 as well when you commit this. > I'll update clang after you commit.Well, I have patches for both (attached). But I am caught between a rock and a hard place, because Xcode 2.4.1-built llvm-gcc ICEs-out on the Release llvm (which is needed as per README-LLVM). So I have a hard time to run those tests. I keep trying. Possibly softlinking the Release dirs to the Debug ones. > > When you commit, please email llvmdev/commits with an email that says > "API CHANGE" in the subject line with info on how to upgrade out-of- > tree projects (e.g. vmkit).Okay, will include the tcsh scripts I have written.> > Thanks Gabor!You are welcome :-) Gabor> > -Chris-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: gcc.diff Type: application/octet-stream Size: 9918 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: clang.diff Type: application/octet-stream Size: 12656 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0001.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0002.html>
On Apr 3, 10:53 pm, Gabor Greif <ga... at mac.com> wrote: ...> > > 3) Make sure that make check and some reasonable subset of llvm-test > > passes with this patch :) > > I have never run llvm-test in the past. Is it just checking it out and > following a readme?After building an llvm-gcc (4.2.1), see below, I tried running this. I configured: ./configure --with-llvmsrc=$LLVM_TOP/llvm --with-llvmobj=$LLVM_TOP/ llvm --srcdir=$LLVM_TOP/test-suite --with-llvmgccdir=$LLVM_TOP/install --with-externals=$LLVM_TOP/externals Then 'make', it complained that some .bc file could not be made. Then I put $LLVM_TOP/install and $LLVM_TOP/llvm/Release/bin im my path, but now checking for C++ compiler default output file name... configure: error: C++ compiler cannot create executables invoking g++ directly gives: ggreif$ g++ jj.cpp /usr/bin/ld: can't locate file for: -lstdc++ I am stuck. What can I do to run the tests?> > > > > Finally, please update llvm-gcc 4.2 as well when you commit this. > > I'll update clang after you commit. > > Well, I have patches for both (attached). But I am caught between a > rock and a > hard place, because Xcode 2.4.1-built llvm-gcc ICEs-out on the > Release llvm (which is needed as per README-LLVM). So I have a hard > time to run those tests. I keep trying. Possibly softlinking the > Release dirs to the Debug ones.This approach works. I get past bootstrapping llvm-gcc. Thanks and cheers, Gabor> > >
heisenbug wrote:> On Apr 3, 10:53 pm, Gabor Greif <ga... at mac.com> wrote: > ... > > >>> 3) Make sure that make check and some reasonable subset of llvm-test >>> passes with this patch :) >>> >> I have never run llvm-test in the past. Is it just checking it out and >> following a readme? >> > > > After building an llvm-gcc (4.2.1), see below, I tried running this. I > configured: > > > ./configure --with-llvmsrc=$LLVM_TOP/llvm --with-llvmobj=$LLVM_TOP/ > llvm --srcdir=$LLVM_TOP/test-suite --with-llvmgccdir=$LLVM_TOP/install > --with-externals=$LLVM_TOP/externals > > Then 'make', it complained that some .bc file could not be made. > > Then I put $LLVM_TOP/install and $LLVM_TOP/llvm/Release/bin im my > path, but now > > checking for C++ compiler default output file name... configure: > error: C++ compiler cannot create executables > > invoking g++ directly gives: > > > ggreif$ g++ jj.cpp > /usr/bin/ld: can't locate file for: -lstdc++ > > I am stuck. What can I do to run the tests? >Did you do a make install? It is probably searching for libstdc++ from the g++ you just built. I usually add a "--program-prefix=llvm-" to configure, and then I can just do a make install into /usr/local without interfering with system's gcc/g++. But for running llvm-test I think you should have system's gcc/g++ on your path and not llvm-gcc/llvm-g++. If I understand correctly, the purpose of the testsuite is to test system gcc/g++ vs. LLC vs. CBE vs. JIT. Best regards, --Edwin
On Thu, 3 Apr 2008, Gabor Greif wrote:>> have consistent use of ::Create for all IR classes to make it easier >> to learn the API. That said, a temporary moment of inconsistency is >> ok: you could commit this part, then switch the remaining classes over. > > > Yeah, this is a wart, but it is only temporary as you say. > I already have some sed scripts that aid the transition :-)No problem.>> 1) Please (eventually) don't make the 'operator new' override be >> public. I'd actually prefer all memory alloc/dealloc stuff to be done >> privately to the (vmcore) .cpp files, not exposed through the header. > > Okay. I guess these relpacement 'operator new' will go away in favor of > Foo::Create methods.yep.>> 2) Eventually we'll need to make the dtor private as well. > > Okay. So no more 'delete xxx;". But then we must be careful to > never call the Foo::Destroy() method on a NULL ptr.I'd suggest making foo::Destroy() be non-virtual. In the implementation it can just check for 'this == null' and return immediately if so. Doing this also helps with the "eliminate the vtable pointer" subproject to shrink "Value".>> 3) Make sure that make check and some reasonable subset of llvm-test >> passes with this patch :) > > I have never run llvm-test in the past. Is it just checking it out and > following a readme?It's pretty easy, take a look at docs/TestingGuide.html>> Finally, please update llvm-gcc 4.2 as well when you commit this. >> I'll update clang after you commit. > > Well, I have patches for both (attached). But I am caught between a rock > and a hard place, because Xcode 2.4.1-built llvm-gcc ICEs-out on the > Release llvm (which is needed as per README-LLVM). So I have a hard time > to run those tests. I keep trying. Possibly softlinking the Release dirs > to the Debug ones.This is strange.>> When you commit, please email llvmdev/commits with an email that says >> "API CHANGE" in the subject line with info on how to upgrade out-of- >> tree projects (e.g. vmkit). > > Okay, will include the tcsh scripts I have written.Nice! -Chris -- http://nondot.org/sabre/ http://llvm.org/
As promised here comes the algorithmic part of the project. I have documented the way how the User object can be recovered from an array of Use objects. I have included a reference implementation in Haskell, along with a randomized test suite, which passes. This is just for those who want to manually prove the correctness of the C++ algorithm. If you wish I can remove (or move to another location) the Haskell part. A new file, Use.cpp contains the codec for the bit-encoding. These routines will be called by the memory allocation routines for User and by Use::getUser(). The interface is not finalized yet, I am thinking of a User <--> Use friendship to make the new routines private, and we definitely need some casting to get this done. I hope to nicely hide the ugliness behind a friendly facade. When this part is done and checked in to trunk, I shall begin to implement the proposed memory layout, but without the memory reduction for now. Instead the two algorithms will run in parallel, backing up each other and comparing the data, asserting on discrepancies. Here is the review link: <http://llvm.org/viewvc/llvm-project?view=rev&revision=49380> Cheers, Gabor
On Apr 8, 3:55 pm, heisenbug <ggr... at gmail.com> wrote:> As promised here comes the algorithmic part of the > project. > > I have documented the way how the User object can be > recovered from an array of Use objects. >Thinking about it, it is probably a poor decision to put the tag bits into the Value*. This incurs unnecessary overhead as the Use::Val member is accessed frequently. My final proposal will say that the Prev member will be used to carry the tag bits necessary for recovering the User*. This member is far less frequently accessed and syntactically better isolated. Cheers, Gabor